Fix invalid memory access. #246
Conversation
http_parse_host() depends on u->field_data[UF_HOST], but this if() allowed the method to be called if only u->field_data[UF_SCHEMA] was set, resulting in use of unintialized pointers.
|
This patch fixed a recurring crash in Phusion Passenger (issue report). |
This update restores that functionality while maintaining the memory access fix (only call http_parse_host if UF_HOST is set).
|
The thread for 225 is related. However, it seems more logical to protect http_parse_host() from being called if the UF_HOST field data is not set anyway than to memset the structure. Either way the patches are compatible and can both be applied for a robust fix of the invalid memory access issue. |
| if (http_parse_host(buf, u, found_at) != 0) { | ||
| return 1; | ||
| } | ||
| if ((u->field_set & (1 << UF_SCHEMA)) && (u->field_set & (1 << UF_HOST)) == 0) { |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
Looks good, but what about failing test? ;) |
http_parse_host also affects UF_PORT and cannot be moved below the check
|
@indutny oh sorry, I thought I was being smart by moving the host and port checks together, but the http_parse_host also parses the port and needed to go back up. Test works for me now, and I also addressed the 80 char limit. |
| @@ -2384,7 +2389,7 @@ http_parser_parse_url(const char *buf, size_t buflen, int is_connect, | |||
|
|
|||
| /* CONNECT requests can only contain "hostname:port" */ | |||
| if (is_connect && u->field_set != ((1 << UF_HOST)|(1 << UF_PORT))) { | |||
| return 1; | |||
| return 1; | |||
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
Looks awesome! May I ask you to submit some kind of test for it? |
|
I checked that the test suite reports a failure with the old code and OK with the new code. |
|
Aaah, I see :) Thanks. |
http_parse_host() depends on `u->field_data[UF_HOST]`, but this if() allowed the method to be called if only `u->field_data[UF_SCHEMA]` was set, resulting in use of unintialized pointers. PR-URL: #246 Reviewed-By: Fedor Indutny <fedor@indutny.com>
|
Landed in f6f436a, thank you! |
|
👍 |
http_parse_host() depends on u->field_data[UF_HOST], but this if()
allowed the method to be called if only u->field_data[UF_SCHEMA] was
set, resulting in use of unintialized pointers.