fix(validators): reject loopback/private/link-local hosts in IsValidRemoteURL#1470
Open
amitvijapur wants to merge 1 commit into
Open
Conversation
…emoteURL IsValidRemoteURL only compared the hostname against the literal strings "localhost", "127.0.0.1", and "*.localhost", so it missed every other notation for an internal address: IPv6 loopback ([::1]), the rest of 127.0.0.0/8, unspecified addresses (0.0.0.0, [::]), IPv4-mapped IPv6 loopback ([::ffff:127.0.0.1]), and RFC1918/link-local ranges including the 169.254.169.254 cloud metadata endpoint. Parse the hostname with net.ParseIP and reject it if IsLoopback, IsUnspecified, IsPrivate, or IsLinkLocalUnicast is true, so every IP notation for these ranges is caught consistently. DNS names (which can't be classified without a network round trip) still fall back to the existing "localhost" / "*.localhost" string checks. Fixes modelcontextprotocol#1465.
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.
Fixes #1465
Cause
IsValidRemoteURLrejected only the literal stringslocalhost/127.0.0.1/*.localhost, so every other notation for a loopback or internal host passed validation:[::1], other127.0.0.0/8addresses (127.0.0.2), unspecified addresses (0.0.0.0,[::]), IPv4-mapped IPv6 forms ([::ffff:127.0.0.1]), and RFC1918/link-local addresses.Change
Extracted the host check into
isDisallowedRemoteHost: if the hostname parses as an IP (net.ParseIP— this includes bracketed IPv6 forms, whichurl.URL.Hostname()already strips, and IPv4-mapped forms), rejectIsLoopback() || IsUnspecified() || IsPrivate() || IsLinkLocalUnicast(). Hostnames that don't parse as an IP keep the pre-existinglocalhost/*.localhoststring checks, since resolving DNS names would require a network round trip during validation.Tests
New table-driven
TestIsValidRemoteURL_LoopbackAndPrivateAddressesininternal/validators/utils_test.go(20 subtests) covering every bypass listed in the issue plus sanity checks that public hosts still validate.go build ./...,go vet ./..., andgofmtall clean;golangci-lint runreports no new findings versusmainon the touched package.