Skip to content

[HttpClient] Fix checking for private IPs before connecting #59013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues -
License MIT

The patch for CVE-2024-50342 isn't enough.

As reported by @cs278:

A DNS name which presents multiple A records can be used to bypass the new fix due to a TOCTOU
condition where the decorator is performing the DNS lookup but curl is then performing its own
DNS query and getting a private IP address.

In addition:

A DNS record which presents a public A record and a private AAAA record can be used to workaround
the protection measures if certain conditions are present

As a first reminder, NoPrivateNetworkHttpClient DOES prevent requests to unwanted servers, but it doesn't prevent connecting to them, allowing to use timings to scan private network topologies.

As a second reminder, according to our security policy, we shouldn't have considered this as a security issue in the first place, but as a security hardening that could be handled without a CVE. Yet we did so we're going to update the already published CVE/advisory/blog post after this PR.

This patch rewrites NoPrivateNetworkHttpClient to make it responsible for resolving DNS and following redirections. I tried using CURLOPT_RESOLVE instead, but this setting is ignored during a curl transaction apparently so we have no other choices.

In addition, the decorator will refuse to connect to IPv6/IPv4 hosts if the configured $subnets doesn't block any IPs for the corresponding version of the protocol.

Because the DNS check is made earlier, the decorator now also rejects relative URLs when the base URI is unknown to it.

@carsonbot carsonbot added this to the 5.4 milestone Nov 27, 2024
@nicolas-grekas nicolas-grekas merged commit 4677f32 into symfony:5.4 Nov 27, 2024
11 of 12 checks passed
@nicolas-grekas nicolas-grekas deleted the hc-pre-resolve branch November 27, 2024 11:57
@fabpot fabpot mentioned this pull request Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants