Repository navigation
Honor the fromURL graceful-failure contract for non-IPv6 hosts - #218
Merged
Merged
Conversation
Address6.fromURL() is documented and typed to return
`{ error, address: null, port: null }` when a URL can't be parsed, but
that contract was only implemented on the regex-miss path. The URL host
character class `[0-9a-f:.]` is a superset of valid IPv6, so any host
made only of hex digits, colons and dots matched the regex and was
handed straight to `new Address6(host)`, whose AddressError propagated
out uncaught.
IPv4 literals are the visible case — `http://127.0.0.1/`,
`http://169.254.169.254/`, `http://0.0.0.0/` all threw — but so did
`[:::]`, `[1:2:3:4:5:6:7:8:9]` and `[::ffff:999.1.1.1]`. A plain
hostname like `example.com` avoided the throw only because letters
outside a-f break the regex match first.
Catch the constructor and return the same error object the regex-miss
path returns, so the function never throws for any input.
Reported by @zikk090 via GHSA-mxvh-v779-f36j. Declining that as a
security advisory: an uncaught synchronous throw from a parser on the
caller's own stack is a correctness bug, not a denial of service.
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
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.
Address6.fromURL()is documented and typed to return{ error, address: null, port: null }when a URL can't be parsed, but that contract was only implemented on the regex-miss path. The URL host character class[0-9a-f:.]is a superset of valid IPv6, so any host made only of hex digits, colons and dots matched the regex and was handed straight tonew Address6(host), whoseAddressErrorpropagated out uncaught.IPv4 literals are the visible case:
but so did
[:::],[1:2:3:4:5:6:7:8:9]and[::ffff:999.1.1.1]. A plain hostname likeexample.comavoided the throw only incidentally — letters outsidea-fbreak the regex match before the constructor is ever reached.The fix
Both regex-miss paths already returned the documented error object, so this hoists that message into a variable and reuses it when the constructor rejects a host the regex accepted.
fromURLnow never throws, for any input.History
This is only partly a regression. The unbracketed IPv4 throw has been there since 7.0.0; before that, 6.4.0 silently parsed
127.0.0.1as the address127, returning wrong data rather than erroring. The bracketed form (http://[127.0.0.1]:80/) did return the graceful error object until 10.2.1, which is the first release carrying #202 — that PR widened the host character class from[0-9a-f:]to[0-9a-f:.]so v4-in-v6 hosts like[::ffff:1.2.3.4]would parse, and.also let IPv4 literals through to the constructor.Tests
Seven new cases in
test/functionality-v6-test.ts: IPv4 host, bracketed IPv4 with port, bare IPv4,[:::], nine groups, out-of-range v4-in-v6 octets, plus a table of eleven hosts assertingfromURLnever throws. All seven fail before the fix. The existing valid-address and v4-in-v6 cases are untouched.Reported by @zikk090 via GHSA-mxvh-v779-f36j, declined as an advisory and fixed as a correctness bug.