Skip to content

Make is_ipaddress detect non-standard forms of IPv4 addresses - #5029

Merged
illia-v merged 2 commits into
urllib3:mainfrom
illia-v:non-standard-ipv4
May 18, 2026
Merged

illia-v merged 2 commits into
urllib3:mainfrom
illia-v:non-standard-ipv4

Conversation

@illia-v

@illia-v illia-v commented May 17, 2026

Copy link
Copy Markdown
Member

@christos-spearbit highlighted that urllib3 can connect to hosts represented as non-standard IPv4 addresses via socket.connect (e.g., urllib3.request("GET", "http://2130706433:8000/")), but our is_ipaddress function does not detect such forms so some logic of the library that relies on is_ipaddress may be missed.

I update the _IPV4_RE regex to match the forms too.

FYI, non-standard IPv4 forms are not allowed inside IPv4-mapped IPv6 addresses like ::ffff:192.168.1.1, so urllib3's logic for them is aligned with RFCs.

@shazow

shazow commented May 17, 2026

Copy link
Copy Markdown
Member

+0

Is it desirable to pass through ambiguous formats straight to socket.connect?

I don't feel strongly here, but just wanted to note that the stdlib ipaddress.ip_address(address) also rejects these kinds of addresses.

Hypothetically, I can imagine some years from now a CVE where something is relying on filtering out some specific addresses is getting bypassed by rewriting those addresses with non-standard variants. 🤪

@illia-v

illia-v commented May 17, 2026

Copy link
Copy Markdown
Member Author

Is it desirable to pass through ambiguous formats straight to socket.connect?

I just don't feel like we need to make a breaking change - disallow such forms of IPv4. But if we keep support for them, the function has to aligned.

a CVE where something is relying on filtering out some specific addresses is getting bypassed by rewriting those addresses with non-standard variants. 🤪

My next change will be related to this 😄. I want to update the security policy to mention that vulnerability reports about SSRF will not be accepted because urllib3 does not provide the functionality.
I know that @sethmlarson usually suggests relying on allow lists instead of block lists as a best practice.

@pquentin pquentin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM.

@illia-v
illia-v merged commit c2a56d9 into urllib3:main May 18, 2026
68 of 69 checks passed
@illia-v
illia-v deleted the non-standard-ipv4 branch May 18, 2026 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants