ipnet
ipnet copied to clipboard
extended version of `std::net::parser` does not include CVE fix
It looks like the copied and extended version of the std::net::parser
module in the ipnet parser doesn't include this recent CVE fix to the standard library that disallows the use of octal format in IPv4 strings: https://github.com/rust-lang/rust/pull/83652.
From the PR to rust-lang/rust:
In its original specification, leading zero in Ipv4 string is interpreted as octal literals. So a IP address 0127.0.0.1 actually means 87.0.0.1.
This confusion can lead to many security vulnerabilities. Therefore, in IETF RFC 6943, it suggests to disallow octal/hexadecimal format in Ipv4 string all together.
If I understand correctly, similarly to std::net::parser
, it's not that a leading zero would cause the string to be interpreted as an octal literal in ipnet's parser, as the parser specifies the radix as 10 here; however, it would be good to fully disallow leading-zero octal format in an IPv4 string as suggested in the above RFC, since it's invalid in the strict format.
Would it make sense to apply that change to ipnet? I'm happy to put together a PR.
Oops, I wrote a test and it looks like the ipnet parser already doesn't allow octal or hex format.
Sorry I missed the original notification about this last week. Thanks for spotting this and working on it. I’m also a bit surprised it doesn’t have the same problem since it was copied from the std lib (but it was long ago) :)
Yeah, that's interesting. I have a theory:
- in the ipnet copy, when reading an octet in an IPv4 address,
max_digits
is specified as 3, which would have correctly triggered the error case in the test I wrote ("0127.0.0.1"
) - whereas in std,
max_digits
is unspecified, so that particular case (four digits with a leading zero) was parsed incorrectly without a manual check until the fix
That might mean that ipnet
is still "exposed" to the CVE in the case of an octet with 3 or fewer digits and a leading zero (e.g. 01.2.3.4
).
Ah, so it would seem. Looks like the parser in std has had quite a few other changes too. Is this something you'd still like to submit a PR for? I'm happy to merge a fix. There's no rush.