An out-of-range -P proxy port parses to a garbage port instead of being rejected#610
Merged
Conversation
hts_parse_proxy read the port with sscanf(a, "%d", &p), which is signed overflow UB past INT_MAX. glibc wraps rather than fails, so a garbage port got through: -P 'http://host:99999999999999' parsed to port 276447231, while 'http://host:4294967296' happened to fail and fall back to the default. 65536 was accepted as-is. Parse with strtol and range-check to *DIGIT in 1..65535 (RFC 3986); an out-of-range or malformed port now falls back to proxy_default_port, exactly as an unparsable one already did. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
RFC 3986 defines port as *DIGIT with no range; the 1..65535 bound comes from TCP's 16-bit port field. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
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
1 participant
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.
hts_parse_proxyread the proxy port withsscanf(a, "%d", &p). A value that doesn't fit anintis undefined behavior there (C11 7.21.6.2p10), and glibc wraps instead of failing, so-P 'http://host:99999999999999'connected to port 276447231 while-P 'http://host:4294967296'happened to fail and fall back to the default. 65536 was taken at face value. The port now goes through strtol with a 1..65535 range check, so anything out of range or malformed falls back toproxy_default_port, exactly as an unparsable port already did. RFC 3986 allows any digit string; the 1..65535 bound is TCP's 16-bit port field.Two smaller behavior changes come with it, both reachable only from a malformed
-P. A port with trailing garbage is now rejected, so-P 'host:80x'defaults instead of silently connecting to 80. A leading+or space is rejected too. A leading-already was.The self-test covers the range fence and the overflow values from the issue, plus
4294967376. That one wraps to a plausible port 80, so it catches a fix that range-checks the sscanf result instead of rejecting the overflow. Six of the fourteen cases discriminate against a buggy build: the fence on both sides, trailing garbage, leading space, the socks5 default, and4294967376. The rest already pass on master and pin the behavior.Closes #602