Bug #103129
closed
Modified "Host" header with invalid port leads to exception when creating the ServerRequestFactory->fromGlobals
Added by Johannes Schlier 9 months ago. Updated 3 days ago.
100%
Description
When a request is passed to TYPO3 that has an invalid port within the "Host" header (e. g. a non-numeric port "aaa" or too many numerals "123123123") TYPO3 pretty quickly runs into an error and returns an exception, including an HTTP 500 error.
In my opinion it would be preferable to silently ignore those invalid settings and either continue the request normally, or at least return a clean error (400 - Bad Request).
The attached screenshots are the complete stack trace of such a request.
This was reproduced with TYPO3 12.4.10 and 12.4.11.
Files
| Screenshot 2024-02-15 at 12.39.31.png (603 KB) Screenshot 2024-02-15 at 12.39.31.png | Johannes Schlier, 2024-02-15 11:45 | ||
| Screenshot 2024-02-15 at 12.39.11.png (601 KB) Screenshot 2024-02-15 at 12.39.11.png | Johannes Schlier, 2024-02-15 11:45 |
Updated by Benni Mack 9 months ago
Idea is to catch invalid argument exception in AbstractApplication->run (dev-main), and then deliver a 400 response with no content.
Updated by Stefan Bürk 7 months ago
- Status changed from New to Needs Feedback
- Assignee set to Stefan Bürk
How did you managed to get this down to TYPO3 (or a PHP application). I cannot reproduce
this in anyway with differnt browsers or curl (cli) directly. neither with direct local
LAMP setups nor ddev (like it seems you are using) - neighter with APACHE2 or NGNIX ?!
Literally, a invalid port part in a URI should be handled
1) by the client
2) by the webserver
Usually, the webserver could not be reached with a invalid port as it should not be
listening on that port. So even if the client does not handle it, it should be handled
on the webserver level. If a Webserver catches this, maybe because of the `https` protocol
in the uri IT still must decline the request because of the invalid port part - or remove
it when passing further down (stripping it from request uri).
So I'm with you - it should be corrected or a 400 Bad Request thrown, but that is not the
task of the PHP application. There is something badly wrong before the application ever
should be reached with such a port.
Silenty handling it on the TYPO3 side would only hide the "server configuration" issue.
I'm really interessted how you could pass such a invalid uri down to TYPO3.
Beside that, I'm with benni that we can add a 400 Bad Request for that case in a early
state, but silently passing it down would open for a lot of other issues anyway. Will
have a look the next day for a implementation.
Can you provide more information how you could triggered that at all ? Operating System,
browser, ddev config etc. ?
Updated by Gerrit Code Review 7 months ago
- Status changed from Needs Feedback to Under Review
Patch set 1 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/83948
Updated by Gerrit Code Review 7 months ago
Patch set 2 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/83948
Updated by Gerrit Code Review 7 months ago
Patch set 3 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/83948
Updated by Gerrit Code Review 7 months ago
Patch set 4 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/83948
Updated by Gerrit Code Review 7 months ago
Patch set 5 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/83948
Updated by Gerrit Code Review 7 months ago
Patch set 6 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/83948
Updated by Stefan Bürk 7 months ago
- Related to Bug #98264: Logging "unsupported" HTTP request methods as an exception into the log is wrong added
Updated by S P 7 months ago
@Stefan Bürk PHP also provides a dedicated built-in webserver. Start it with "php -S". Then your PHP code must handle these cases.
Updated by Stefan Bürk 7 months ago
· Edited
Stefan P wrote in #note-10:
@Stefan Bürk PHP also provides a dedicated built-in webserver. Start it with "php -S". Then your PHP code must handle these cases.
Sidenote: PHP -S is not an official supported webserver nor a production ready webserver. See https://www.php.net/manual/en/features.commandline.webserver.php
Warning: This web server is designed to aid application development. It may also be useful for testing purposes or for application demonstrations that are run in controlled environments. It is not intended to be a full-featured web server. It should not be used on a public network.
Even with that information - php build-in webserver won't start with such an invalid port, and I still could not find a way or client to "reach" port 443 or 80 or any other valid port where a webserver is listening) and having a manipulated header with a wrong port in it. I search for a way to create such a malformed request.
The related issue was about a invalid HTTP method - which can be done, for example using curl. But for an invalid port ?
I created a change - but detected that another (WIP) change from 2022 is hanging around with a literlly better approach.
Reading through it, sub-requests are mentioned which make literally sense to mitigate issues there also. I pinged the
author and core mergers discussion it back then, and we will see how to proceed here. I already linked the other issue
and with that the other change.
Updated by Benni Mack 6 months ago
Stefan (Buerk),
tbh, I have seen this issue in production AND with "the recommended local dev environment for TYPO3" — DDEV, with unmodified changes (nginx). Here is an example:
curl \ --compressed \ -H "Accept: */*" \ -H "Accept-Encoding: gzip" \ -H "Content-Length: " \ -H "Content-Type: " \ -H "Host: mysite.ddev.site:benni" \ -H "User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/93.0.4577.82 Safari/537.36" \ "https://mysite.ddev.site/?ttrp672686=ttrp151800"
I even tried some domains I will not mention in this document. I guess this is related to the default nginx configuration that has been documented for decades.
Updated by Markus Klein about 2 months ago
- Related to Task #105163: Do not log Exceptions due to invalid client headers added
Updated by Markus Klein about 2 months ago
- Related to Bug #105194: Lots of log entries for invalid http requests added
Updated by Gerrit Code Review about 2 months ago
Patch set 6 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/86416
Updated by Gerrit Code Review about 1 month ago
Patch set 7 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/83948
Updated by Gerrit Code Review about 1 month ago
Patch set 7 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/86416
Updated by Gerrit Code Review 10 days ago
Patch set 8 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/86416
Updated by Gerrit Code Review 7 days ago
Patch set 9 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/86416
Updated by Gerrit Code Review 3 days ago
Patch set 1 for branch 13.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/87181
Updated by Gerrit Code Review 3 days ago
Patch set 1 for branch 12.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/87182
Updated by Markus Klein 3 days ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset d21846771aa9c8286b5de2e17c4b76637142a2fa.
Updated by Gerrit Code Review 3 days ago
- Status changed from Resolved to Under Review
Patch set 2 for branch 12.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/87182
Updated by Markus Klein 3 days ago
- Status changed from Under Review to Resolved
Applied in changeset 4148f82e9e800f725746f43774f57592e802a1fa.