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

Ipv4Addr: Incorrect Parsing for Octal format IP string #83648

Closed
xu-cheng opened this issue Mar 29, 2021 · 3 comments · Fixed by #83652
Closed

Ipv4Addr: Incorrect Parsing for Octal format IP string #83648

xu-cheng opened this issue Mar 29, 2021 · 3 comments · Fixed by #83652
Labels
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

@xu-cheng
Copy link
Contributor

xu-cheng commented Mar 29, 2021

This issue is inspired by this blog.

Due to the specification, leading zero in IP string is interpreted as octal literals. So a IP address 0127.0.0.1 actually means 87.0.0.1. As shown in the following example:

❯ ping 0127.0.0.1
PING 0127.0.0.1 (87.0.0.1): 56 data bytes

However, the Ipv4Addr from the std library will recognize it as 127.0.0.1 instead. A simple code to demo the situation (playground link):

use std::net::Ipv4Addr;

fn parse(input: &str) {
    let ip: Option<Ipv4Addr> = input.parse().ok();
    println!("{} -> {:?}", input, ip);
    
}

fn main() {
    parse("127.0.0.1");
    parse("0127.0.0.1");
}

I expected to see this happen:

127.0.0.1 -> Some(127.0.0.1)
0127.0.0.1 -> Some(87.0.0.1)

Instead, this happened:

127.0.0.1 -> Some(127.0.0.1)
0127.0.0.1 -> Some(127.0.0.1)

Noted this bug may cause security vulnerabilities in certain cases. For example, a Rust program uses Ipv4Addr doing some sanity check then passing the user string to other library or program.

Furthermore, the specification actually also allows hex format in IP string.

Meta

rustc --version --verbose:

rustc 1.51.0 (2fd73fabe 2021-03-23)
@xu-cheng xu-cheng added the C-bug Category: This is a bug. label Mar 29, 2021
@jonas-schievink jonas-schievink added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 29, 2021
@xu-cheng
Copy link
Contributor Author

While the fix should be quite straightforward, there are two possible solutions:

@joshtriplett
Copy link
Member

It's not at all obvious that we should parse octal IP addresses. Seems exceedingly unlikely to come up outside of security advisories. I would venture a guess that if we added this, far more people would be tripped up by it happening unexpectedly than would ever use it intentionally.

@xu-cheng
Copy link
Contributor Author

I would venture a guess that if we added this, far more people would be tripped up by it happening unexpectedly than would ever use it intentionally.

I agree. So I think disallowing octal string like inet_pton should be better approach.

Nevertheless, the current implementation in Rust std library should be considered as a (low-risk?) security vulnerability.

bors added a commit to rust-lang-ci/rust that referenced this issue Mar 30, 2021
Disallow octal format in Ipv4 string

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.

Existing implementation already disallows hexadecimal numbers. This commit
makes Parser reject octal numbers.

Fixes rust-lang#83648.

[IETF RFC 6943]: https://tools.ietf.org/html/rfc6943#section-3.1.1
@bors bors closed this as completed in 974192c Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

Successfully merging a pull request may close this issue.

3 participants