Skip to content
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

Some valid IPv4 addresses are not parsed by Ipv4Addr::from_str() #90199

Open
linkmauve opened this issue Oct 23, 2021 · 5 comments
Open

Some valid IPv4 addresses are not parsed by Ipv4Addr::from_str() #90199

linkmauve opened this issue Oct 23, 2021 · 5 comments
Labels
A-io Area: std::io, std::fs, std::net and std::path C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@linkmauve
Copy link
Contributor

linkmauve commented Oct 23, 2021

An IPv4 address is usually represented by four decimal numbers separated by a dot, but this isn’t the only allowed representation. Most programs support alternative representations with fewer dots where the last number is either 16-bit, 24-bit, or 32-bit, see the manpage of inet_aton(). In order to be compatible with these programs and users’ assumptions, std::net::Ipv4Addr should add support for these formats.

I tried this code:

use std::net::Ipv4Addr;
fn main() {
    let localhost: Ipv4Addr = "127.1".parse().unwrap();
    assert_eq!(format!("{}", localhost), "127.0.0.1");
}

I expected to see this happen: nothing, as the parsing would succeed and then the assert would pass.

Instead, this happened: thread 'main' panicked at 'called Result::unwrap() on an Err value: AddrParseError(())', src/main.rs:3:47

Meta

rustc --version --verbose:

rustc 1.58.0-nightly (547a6ffee 2021-10-21)
binary: rustc
commit-hash: 547a6ffee0cf4da9929a9e3d49546dc87d607735
commit-date: 2021-10-21
host: x86_64-unknown-linux-gnu
release: 1.58.0-nightly
LLVM version: 13.0.0
Backtrace

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: AddrParseError(())', src/main.rs:4:45
stack backtrace:
   0: rust_begin_unwind
             at /rustc/514b3877956dc594823106b66c164f8cdbc8b3da/library/std/src/panicking.rs:495:5
   1: core::panicking::panic_fmt
             at /rustc/514b3877956dc594823106b66c164f8cdbc8b3da/library/core/src/panicking.rs:107:14
   2: core::result::unwrap_failed
             at /rustc/514b3877956dc594823106b66c164f8cdbc8b3da/library/core/src/result.rs:1617:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/514b3877956dc594823106b66c164f8cdbc8b3da/library/core/src/result.rs:1299:23
   4: playground::main
             at ./src/main.rs:4:29
   5: core::ops::function::FnOnce::call_once
             at /rustc/514b3877956dc594823106b66c164f8cdbc8b3da/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

@linkmauve linkmauve added the C-bug Category: This is a bug. label Oct 23, 2021
@the8472
Copy link
Member

the8472 commented Oct 23, 2021

This is intentional for security purposes. As the documentation says:

Ipv4Addr provides a FromStr implementation. The four octets are in decimal notation, divided by . (this is called “dot-decimal notation”). Notably, octal numbers and hexadecimal numbers are not allowed per IETF RFC 6943.

Which links to the section of RFC 6943 which describes strict parsing of IPv4 literals.

@Stargateur
Copy link
Contributor

That not for security reason, it's just a ambiguous syntax, and nobody use it. Ipv6 :: make it no ambiguous.

The security in doc is for octal confusion like 042.42.42.42

@hkratz
Copy link
Contributor

hkratz commented Oct 23, 2021

The doc is a bit incomplete/imprecise IMHO as the section of the RFC defines a strict and loose form. We should reword it thus:

Ipv4Addr provides a [FromStr] implementation. The address must be in "dot-decimal notation", consisting of four decimal numbers between 0 and 255, divided by . This is also referred to as the "strict form of an IPv4 address literal" in section 3.1.1 of IETF RFC 6943.

daladim pushed a commit to JustRustThings/xmpp-rs that referenced this issue Feb 15, 2022
This makes the tests pass again on nightly, and avoids using legacy IP
while we have glorious IPv6 support everywhere nowadays.

See also rust-lang/rust#90199
@ChrisDenton ChrisDenton added the needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. label Jul 16, 2023
@dkasak
Copy link

dkasak commented Oct 16, 2023

Note that the current implementation leaves open the possibility of incorrectly considering a string not to be an IPv4 address literal when it actually is one.

Imagine if this were used in validation code to prevent connections to raw IP addresses: it could easily offer a validation bypass since low-level networking functions will happily connect to it.

@ChrisDenton ChrisDenton added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. A-io Area: std::io, std::fs, std::net and std::path and removed needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. labels Oct 16, 2023
@ChrisDenton
Copy link
Contributor

Tbh, I personally think this is just a documentation bug. Rust has chosen to use only the strict form when parsing. We should be clearer about documenting that and also document any caveats that people should be aware of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-io Area: std::io, std::fs, std::net and std::path C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants