Fix out-of-bounds read on empty Location header in HTTP wrapper - #23467
Open
iliaal wants to merge 1 commit into
Open
Fix out-of-bounds read on empty Location header in HTTP wrapper#23467iliaal wants to merge 1 commit into
iliaal wants to merge 1 commit into
Conversation
laruence
reviewed
Aug 26, 2026
| char *loc_path = NULL; | ||
| if (*header_info.location != '/') { | ||
| if (*(header_info.location+1) != '\0' && resource->path) { | ||
| if (header_info.location_len > 0 && *(header_info.location+1) != '\0' && resource->path) { |
Member
There was a problem hiding this comment.
hmm, this might not be related to your PR, but since it already have location_len, why it still call strlen(location) above?
Contributor
Author
There was a problem hiding this comment.
Switched to location_len.
Member
There was a problem hiding this comment.
do you still need *(header_info.location+1) != '\0' tough ?
Contributor
Author
There was a problem hiding this comment.
No. Empty Location is length 0. A 1-char relative path is valid, so location_len > 0 is enough.
iliaal
added a commit
to iliaal/php-src
that referenced
this pull request
Aug 26, 2026
An empty Location header allocates a single byte for the NUL terminator, so reading location[1] in the relative-redirect branch over-reads heap memory and could append a garbage-derived path to the redirect target instead of the correct host root. Guard the read with location_len before dereferencing the second byte, and use that length instead of strlen for the absolute-URL check. Closes phpGH-23467
iliaal
force-pushed
the
fix/http-empty-location-84
branch
from
August 26, 2026 12:22
5e412a6 to
330bb1e
Compare
iliaal
added a commit
to iliaal/php-src
that referenced
this pull request
Aug 26, 2026
An empty Location header allocates a single byte for the NUL terminator, so reading location[1] in the relative-redirect branch over-reads heap memory and could append a garbage-derived path to the redirect target instead of the correct host root. Use location_len instead of strlen, and require location_len > 1 before treating the value as a relative path, so the second byte is never read. Closes phpGH-23467
iliaal
force-pushed
the
fix/http-empty-location-84
branch
2 times, most recently
from
August 26, 2026 20:43
330bb1e to
8a54f0a
Compare
An empty Location header allocates a single byte for the NUL terminator, so reading location[1] in the relative-redirect branch over-reads heap memory and could append a garbage-derived path to the redirect target instead of the correct host root. Use location_len instead of strlen, and skip the relative join when location_len is 0, so the second byte is never read. Closes phpGH-23467
iliaal
force-pushed
the
fix/http-empty-location-84
branch
from
August 26, 2026 20:53
8a54f0a to
231f29f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a server sends a redirect with an empty Location header, the wrapper allocates a single byte for it and the relative-redirect branch then reads location[1], one byte past the allocation, so a hostile server can make the over-read pick up heap garbage and turn the redirect target into the current path plus junk instead of the host root. The second-byte dereference is now guarded by header_info.location_len; an empty Location deterministically redirects to the host root. Sibling audit found no other unguarded indexing of header_info.location.