Bug #72299
closedWrong CRLF's in GeneralUtility for 'Close connection'
Added by D. Röhrig almost 9 years ago. Updated about 6 years ago.
100%
Description
The problem was a bug for a 404 page.
I declared a "pageNotFound_handling" to a certain page ('/index.php?id=6') and the TYPO3 core rendered an extra error report (see image).
After debugging I found the reason in GeneralUtility.php (see patch). The line breaks inside quote signs are different to the variable CRLF and too many.
I hope the created patch is correct.
Files
Bildschirmfoto 2015-12-17 um 14.38.20.png (40.3 KB) Bildschirmfoto 2015-12-17 um 14.38.20.png | Error | D. Röhrig, 2015-12-17 15:10 | |
patch.diff (1.19 KB) patch.diff | Patch | D. Röhrig, 2015-12-17 15:10 | |
patch.diff (1.2 KB) patch.diff | Patch | D. Röhrig, 2015-12-17 16:51 | |
typo3_6.2.30_bug_72299_404_fix.diff (867 Bytes) typo3_6.2.30_bug_72299_404_fix.diff | Patch for TYPO3 6.2.30 | Felix Buenemann, 2017-03-23 16:02 |
Updated by Markus Klein almost 9 years ago
Thanks for your efforts. Can you please push patches to Gerrit?
(If you need help, please get in touch with us on Slack!)
Updated by Oliver Hader almost 9 years ago
See https://wiki.typo3.org/Contribution_Walkthrough_Tutorials for pushing patches to our review system "Gerrit"
Updated by D. Röhrig almost 9 years ago
- File patch.diff patch.diff added
I forgot one CRLF. I will push the patch to Gerrit.
Updated by Gerrit Code Review almost 9 years ago
- Status changed from New to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/45404
Updated by Gerrit Code Review almost 9 years ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/45404
Updated by Gerrit Code Review almost 9 years ago
Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/45404
Updated by Gerrit Code Review almost 9 years ago
Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/45404
Updated by Gerrit Code Review almost 9 years ago
Patch set 1 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/45420
Updated by Markus Klein almost 9 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset fccf65311947382be3442337f502966712a04382.
Updated by Felix Buenemann over 7 years ago
This is a critical bugfix that needs to be back-ported to 6.2 LTS.
We are currently seeing 400 Bad Request errors for all 404 Pages on TYPO3 6.2.30 with PHP 5.6.30 and apache2 2.4.25.
Incorrect headers sent:
GET /index.php?id=error-404 HTTP/1.0\r\n Host: localhost\n \n Connection: close\n \n User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_3) AppleWebKit/602.4.8 (KHTML, like Gecko) Version/10.0.3 Safari/602.4.8\r\n Referer: http://localhost/index.php?id=18\r\n \r\n
Correct headers would look like:
GET /index.php?id=error-404 HTTP/1.0\r\n Host: localhost\r\n Connection: close\r\n User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_3) AppleWebKit/602.4.8 (KHTML, like Gecko) Version/10.0.3 Safari/602.4.8\r\n Referer: http://localhost/index.php?id=18\r\n \r\n
Updated by Felix Buenemann over 7 years ago
I've attached a backported patch for TYPO3 6.2.30 (without whitespace changes).
Updated by Markus Klein over 7 years ago
@Felix: thanks, but the decision of the backport needs to be granted by the core team or the team leader first.
The patch here does not help us, we need that on Gerrit.
Please push your changes to Gerrit and reuse the commit message and change-id of the 7.6 patch.
I can't guarantee that the change will be done at this point though.
Updated by Markus Klein over 7 years ago
Permission granted. We can push the patch.
Updated by Gerrit Code Review over 7 years ago
- Status changed from Resolved to Under Review
Patch set 1 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/52145
Updated by Markus Klein over 7 years ago
- Status changed from Under Review to Resolved
Applied in changeset 11506d7792cdbcb589a6e08f0f8c295cb157614d.
Updated by Felix Buenemann over 7 years ago
@Markus: Thanks for taking care of the backport.
I've also done some digging into why the error only started causing problems recently and it looks to be related to Apache 2.4.25 CVE-2016-8743 security fixes (debian 2.4.25-1 changelog):
* Security: CVE-2016-8743: Enforce HTTP request grammar corresponding to RFC7230 for request lines and request headers, to prevent response splitting and cache pollution by malicious clients or downstream proxies. * The stricter HTTP enforcement may cause compatibility problems with non-conforming clients. Fine-tuning is possible with the new HttpProtocolOptions directive.
There's also some useful info on the Appache httpd 2.4 vulnerabilities page:
Apache HTTP Server, prior to release 2.4.25, accepted a broad pattern of unusual whitespace patterns from the user-agent, including bare CR, FF, VTAB in parsing the request line and request header lines, as well as HTAB in parsing the request line. Any bare CR present in request lines was treated as whitespace and remained in the request field member "the_request", while a bare CR in the request header field name would be honored as whitespace, and a bare CR in the request header field value was retained the input headers array. Implied additional whitespace was accepted in the request line and prior to the ':' delimiter of any request header lines.
RFC7230 Section 3.5 calls out some of these whitespace exceptions, and section 3.2.3 eliminated and clarified the role of implied whitespace in the grammer of this specification. Section 3.1.1 requires exactly one single SP between the method and request-target, and between the request-target and HTTP-version, followed immediately by a CRLF sequence. None of these fields permit any (unencoded) CTL character whatsoever. Section 3.2.4 explicitly disallowed any whitespace from the request header field prior to the ':' character, while Section 3.2 disallows all CTL characters in the request header line other than the HTAB character as whitespace.
These defects represent a security concern when httpd is participating in any chain of proxies or interacting with back-end application servers, either through mod_proxy or using conventional CGI mechanisms. In each case where one agent accepts such CTL characters and does not treat them as whitespace, there is the possiblity in a proxy chain of generating two responses from a server behind the uncautious proxy agent. In a sequence of two requests, this results in request A to the first proxy being interpreted as requests A + A' by the backend server, and if requests A and B were submitted to the first proxy in a keepalive connection, the proxy may interpret response A' as the response to request B, polluting the cache or potentially serving the A' content to a different downstream user-agent.
These defects are addressed with the release of Apache HTTP Server 2.4.25 and coordinated by a new directive;
which is the default behavior of 2.4.25 and later. By toggling from 'Strict' behavior to 'Unsafe' behavior, some of the restrictions may be relaxed to allow some invalid HTTP/1.1 clients to communicate with the server, but this will reintroduce the possibility of the problems described in this assessment. Note that relaxing the behavior to 'Unsafe' will still not permit raw CTLs other than HTAB (where permitted), but will allow other RFC requirements to not be enforced, such as exactly two SP characters in the request line.
Updated by Markus Klein over 7 years ago
Good point. And I actually fully support this. Protocols have to be adhered 100%, full stop. ;-)