extended version of std::net::parser does not include CVE fix

[peterthejohnston]

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: rust-lang/rust#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.

[peterthejohnston]

Oops, I wrote a test and it looks like the ipnet parser already doesn't allow octal or hex format.

[krisprice]

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) :)

[peterthejohnston]

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).

[krisprice]

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.