Disallow octal format in Ipv4 string #83652

xu-cheng · 2021-03-29T17:05:17Z

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 #83648.

rust-highfive · 2021-03-29T17:05:20Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

sfackler · 2021-03-30T00:14:15Z

This seems like a reasonable change to me, but worth an FCP since it changes the behavior here.

@rfcbot fcp merge

rfcbot · 2021-03-30T00:14:16Z

Team member @sfackler has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

library/std/src/net/parser/tests.rs

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

rfcbot · 2021-03-30T05:59:16Z

🔔 This is now entering its final comment period, as per the review above. 🔔

sfackler · 2021-03-30T11:06:04Z

@bors r+

bors · 2021-03-30T11:06:05Z

📌 Commit 974192c has been approved by sfackler

bors · 2021-03-30T19:34:27Z

⌛ Testing commit 974192c with merge 74874a6...

bors · 2021-03-30T22:15:34Z

☀️ Test successful - checks-actions
Approved by: sfackler
Pushing 74874a6 to master...

pthariensflame · 2021-04-08T22:10:46Z

This will need compat-related relnotes, right?

cuviper · 2021-04-08T22:16:48Z

Sure, I've now added the relnotes label.

Pkgsrc changes: * Bump bootstrap requirements to 1.52.1 * Adjust patches, adapt to upstream changes, adjust cargo checksums * If using an external llvm, require >= 10.0 Upsteream changes: Version 1.53.0 (2021-06-17) ============================ Language ----------------------- - [You can now use unicode for identifiers.][83799] This allows multilingual identifiers but still doesn't allow glyphs that are not considered characters such as `#` (diamond) or `<U+1F980>` (crab). More specifically you can now use any identifier that matches the UAX #31 "Unicode Identifier and Pattern Syntax" standard. This is the same standard as languages like Python, however Rust uses NFC normalization which may be different from other languages. - [You can now specify "or patterns" inside pattern matches.][79278] Previously you could only use `|` (OR) on complete patterns. E.g. ```rust let x = Some(2u8); // Before matches!(x, Some(1) | Some(2)); // Now matches!(x, Some(1 | 2)); ``` - [Added the `:pat_param` `macro_rules!` matcher.][83386] This matcher has the same semantics as the `:pat` matcher. This is to allow `:pat` to change semantics to being a pattern fragment in a future edition. Compiler ----------------------- - [Updated the minimum external LLVM version to LLVM 10.][83387] - [Added Tier 3\* support for the `wasm64-unknown-unknown` target.][80525] - [Improved debuginfo for closures and async functions on Windows MSVC.][83941] \* Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries ----------------------- - [Abort messages will now forward to `android_set_abort_message` on Android platforms when available.][81469] - [`slice::IterMut<'_, T>` now implements `AsRef<[T]>`][82771] - [Arrays of any length now implement `IntoIterator`.][84147] Currently calling `.into_iter()` as a method on an array will return `impl Iterator<Item=&T>`, but this may change in a future edition to change `Item` to `T`. Calling `IntoIterator::into_iter` directly on arrays will provide `impl Iterator<Item=T>` as expected. - [`leading_zeros`, and `trailing_zeros` are now available on all `NonZero` integer types.][84082] - [`{f32, f64}::from_str` now parse and print special values (`NaN`, `-0`) according to IEEE RFC 754.][78618] - [You can now index into slices using `(Bound<usize>, Bound<usize>)`.][77704] - [Add the `BITS` associated constant to all numeric types.][82565] Stabilised APIs --------------- - [`AtomicBool::fetch_update`] - [`AtomicPtr::fetch_update`] - [`BTreeMap::retain`] - [`BTreeSet::retain`] - [`BufReader::seek_relative`] - [`DebugStruct::non_exhaustive`] - [`Duration::MAX`] - [`Duration::ZERO`] - [`Duration::is_zero`] - [`Duration::saturating_add`] - [`Duration::saturating_mul`] - [`Duration::saturating_sub`] - [`ErrorKind::Unsupported`] - [`Option::insert`] - [`Ordering::is_eq`] - [`Ordering::is_ge`] - [`Ordering::is_gt`] - [`Ordering::is_le`] - [`Ordering::is_lt`] - [`Ordering::is_ne`] - [`OsStr::is_ascii`] - [`OsStr::make_ascii_lowercase`] - [`OsStr::make_ascii_uppercase`] - [`OsStr::to_ascii_lowercase`] - [`OsStr::to_ascii_uppercase`] - [`Peekable::peek_mut`] - [`Rc::decrement_strong_count`] - [`Rc::increment_strong_count`] - [`Vec::extend_from_within`] - [`array::from_mut`] - [`array::from_ref`] - [`char::MAX`] - [`char::REPLACEMENT_CHARACTER`] - [`char::UNICODE_VERSION`] - [`char::decode_utf16`] - [`char::from_digit`] - [`char::from_u32_unchecked`] - [`char::from_u32`] - [`cmp::max_by_key`] - [`cmp::max_by`] - [`cmp::min_by_key`] - [`cmp::min_by`] - [`f32::is_subnormal`] - [`f64::is_subnormal`] Cargo ----------------------- - [Cargo now supports git repositories where the default `HEAD` branch is not "master".][cargo/9392] This also includes a switch to the version 3 `Cargo.lock` format which can handle default branches correctly. - [macOS targets now default to `unpacked` split-debuginfo.][cargo/9298] - [The `authors` field is no longer included in `Cargo.toml` for new projects.][cargo/9282] Rustdoc ----------------------- - [Added the `rustdoc::bare_urls` lint that warns when you have URLs without hyperlinks.][81764] Compatibility Notes ------------------- - [Implement token-based handling of attributes during expansion][82608] - [`Ipv4::from_str` will now reject octal format IP addresses in addition to rejecting hexadecimal IP addresses.][83652] The octal format can lead to confusion and potential security vulnerabilities and [is no longer recommended][ietf6943]. Internal Only ------------- These changes provide no direct user facing benefits, but represent significant improvements to the internals and overall performance of rustc and related tools. - [Rework the `std::sys::windows::alloc` implementation.][83065] - [rustdoc: Don't enter an infer_ctxt in get_blanket_impls for impls that aren't blanket impls.][82864] - [rustdoc: Only look at blanket impls in `get_blanket_impls`][83681] - [Rework rustdoc const type][82873] [83386]: rust-lang/rust#83386 [82771]: rust-lang/rust#82771 [84147]: rust-lang/rust#84147 [84082]: rust-lang/rust#84082 [83799]: rust-lang/rust#83799 [83681]: rust-lang/rust#83681 [83652]: rust-lang/rust#83652 [83387]: rust-lang/rust#83387 [82873]: rust-lang/rust#82873 [82864]: rust-lang/rust#82864 [82608]: rust-lang/rust#82608 [82565]: rust-lang/rust#82565 [80525]: rust-lang/rust#80525 [79278]: rust-lang/rust#79278 [78618]: rust-lang/rust#78618 [77704]: rust-lang/rust#77704 [83941]: rust-lang/rust#83941 [83065]: rust-lang/rust#83065 [81764]: rust-lang/rust#81764 [81469]: rust-lang/rust#81469 [cargo/9298]: rust-lang/cargo#9298 [cargo/9282]: rust-lang/cargo#9282 [cargo/9392]: rust-lang/cargo#9392 [`char::MAX`]: https://doc.rust-lang.org/std/primitive.char.html#associatedconstant.MAX [`char::REPLACEMENT_CHARACTER`]: https://doc.rust-lang.org/std/primitive.char.html#associatedconstant.REPLACEMENT_CHARACTER [`char::UNICODE_VERSION`]: https://doc.rust-lang.org/std/primitive.char.html#associatedconstant.UNICODE_VERSION [`char::decode_utf16`]: https://doc.rust-lang.org/std/primitive.char.html#method.decode_utf16 [`char::from_u32`]: https://doc.rust-lang.org/std/primitive.char.html#method.from_u32 [`char::from_u32_unchecked`]: https://doc.rust-lang.org/std/primitive.char.html#method.from_u32_unchecked [`char::from_digit`]: https://doc.rust-lang.org/std/primitive.char.html#method.from_digit [`AtomicBool::fetch_update`]: https://doc.rust-lang.org/std/sync/atomic/struct.AtomicBool.html#method.fetch_update [`AtomicPtr::fetch_update`]: https://doc.rust-lang.org/std/sync/atomic/struct.AtomicPtr.html#method.fetch_update [`BTreeMap::retain`]: https://doc.rust-lang.org/std/collections/struct.BTreeMap.html#method.retain [`BTreeSet::retain`]: https://doc.rust-lang.org/std/collections/struct.BTreeSet.html#method.retain [`BufReader::seek_relative`]: https://doc.rust-lang.org/std/io/struct.BufReader.html#method.seek_relative [`DebugStruct::non_exhaustive`]: https://doc.rust-lang.org/std/fmt/struct.DebugStruct.html#method.finish_non_exhaustive [`Duration::MAX`]: https://doc.rust-lang.org/std/time/struct.Duration.html#associatedconstant.MAX [`Duration::ZERO`]: https://doc.rust-lang.org/std/time/struct.Duration.html#associatedconstant.ZERO [`Duration::is_zero`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.is_zero [`Duration::saturating_add`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.saturating_add [`Duration::saturating_mul`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.saturating_mul [`Duration::saturating_sub`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.saturating_sub [`ErrorKind::Unsupported`]: https://doc.rust-lang.org/std/io/enum.ErrorKind.html#variant.Unsupported [`Option::insert`]: https://doc.rust-lang.org/std/option/enum.Option.html#method.insert [`Ordering::is_eq`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.is_eq [`Ordering::is_ge`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.is_ge [`Ordering::is_gt`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.is_gt [`Ordering::is_le`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.is_le [`Ordering::is_lt`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.is_lt [`Ordering::is_ne`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.is_ne [`OsStr::eq_ignore_ascii_case`]: https://doc.rust-lang.org/std/ffi/struct.OsStr.html#method.eq_ignore_ascii_case [`OsStr::is_ascii`]: https://doc.rust-lang.org/std/ffi/struct.OsStr.html#method.is_ascii [`OsStr::make_ascii_lowercase`]: https://doc.rust-lang.org/std/ffi/struct.OsStr.html#method.make_ascii_lowercase [`OsStr::make_ascii_uppercase`]: https://doc.rust-lang.org/std/ffi/struct.OsStr.html#method.make_ascii_uppercase [`OsStr::to_ascii_lowercase`]: https://doc.rust-lang.org/std/ffi/struct.OsStr.html#method.to_ascii_lowercase [`OsStr::to_ascii_uppercase`]: https://doc.rust-lang.org/std/ffi/struct.OsStr.html#method.to_ascii_uppercase [`Peekable::peek_mut`]: https://doc.rust-lang.org/std/iter/struct.Peekable.html#method.peek_mut [`Rc::decrement_strong_count`]: https://doc.rust-lang.org/std/rc/struct.Rc.html#method.increment_strong_count [`Rc::increment_strong_count`]: https://doc.rust-lang.org/std/rc/struct.Rc.html#method.increment_strong_count [`Vec::extend_from_within`]: https://doc.rust-lang.org/beta/std/vec/struct.Vec.html#method.extend_from_within [`array::from_mut`]: https://doc.rust-lang.org/beta/std/array/fn.from_mut.html [`array::from_ref`]: https://doc.rust-lang.org/beta/std/array/fn.from_ref.html [`cmp::max_by_key`]: https://doc.rust-lang.org/beta/std/cmp/fn.max_by_key.html [`cmp::max_by`]: https://doc.rust-lang.org/beta/std/cmp/fn.max_by.html [`cmp::min_by_key`]: https://doc.rust-lang.org/beta/std/cmp/fn.min_by_key.html [`cmp::min_by`]: https://doc.rust-lang.org/beta/std/cmp/fn.min_by.html [`f32::is_subnormal`]: https://doc.rust-lang.org/std/primitive.f64.html#method.is_subnormal [`f64::is_subnormal`]: https://doc.rust-lang.org/std/primitive.f64.html#method.is_subnormal [ietf6943]: https://datatracker.ietf.org/doc/html/rfc6943#section-3.1.1

huangjj27 · 2021-08-07T14:22:07Z

I think this is the proper fix for #61186.

Reject octal zeros in IPv4 addresses This fixes rust-lang#86964 by rejecting octal zeros in IP addresses, such that `192.168.00.00000000` is rejected with a parse error, since having leading zeros in front of another zero indicates it is a zero written in octal notation, which is not allowed in the strict mode specified by RFC 6943 3.1.1. Octal rejection was implemented in rust-lang#83652, but due to the way it was implemented octal zeros were still allowed.

rust-highfive assigned sfackler Mar 29, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 29, 2021

This comment has been minimized.

Sign in to view

xu-cheng force-pushed the ipv4-octal branch from f72679b to f375592 Compare March 29, 2021 17:41

sfackler added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 30, 2021

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Mar 30, 2021

cuviper reviewed Mar 30, 2021

View reviewed changes

library/std/src/net/parser/tests.rs Outdated Show resolved Hide resolved

library/std/src/net/parser/tests.rs Outdated Show resolved Hide resolved

xu-cheng force-pushed the ipv4-octal branch from f375592 to 974192c Compare March 30, 2021 02:24

rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Mar 30, 2021

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 30, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 30, 2021

bors merged commit 74874a6 into rust-lang:master Mar 30, 2021

rustbot added this to the 1.53.0 milestone Mar 30, 2021

xu-cheng deleted the ipv4-octal branch March 31, 2021 01:25

cuviper added the relnotes Marks issues that should be documented in the release notes of the next release. label Apr 8, 2021

rfcbot added the finished-final-comment-period The final comment period is finished for this PR / Issue. label Apr 9, 2021

rfcbot added to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Apr 9, 2021

apiraino removed the to-announce Announce this issue on triage meeting label Apr 15, 2021

mbartlett21 mentioned this pull request Jul 8, 2021

std::net::Ipv4Addr parsing violates the "strict" form in IETF RFC 6943 Section 3.1.1. #86964

Closed

syvb mentioned this pull request Jul 8, 2021

Reject octal zeros in IPv4 addresses #86984

Merged

sickcodes added a commit to sickcodes/security that referenced this pull request Aug 8, 2021

Fixed in rust 1.53: [rust-lang/rust#83652

ba969a3

sickcodes added a commit to sickcodes/security that referenced this pull request Aug 8, 2021

[CVE-2021-29922] Add Fixed in rust 1.53: [rust-lang/rust#83652

b19c374

peterthejohnston mentioned this pull request Aug 26, 2021

extended version of std::net::parser does not include CVE fix krisprice/ipnet#32

Open

orf mentioned this pull request Jan 15, 2022

Octal and hexadecimal ipv4 address parsing orf/gping#144

Closed

jyn514 mentioned this pull request Mar 10, 2022

IpAddr::from_str is unnecessarily slow for IPv6 addresses #94825

Open

ilyvion mentioned this pull request Aug 19, 2022

Ipv4Addr can't parse perfectly valid IP address #100749

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disallow octal format in Ipv4 string #83652

Disallow octal format in Ipv4 string #83652

xu-cheng commented Mar 29, 2021

rust-highfive commented Mar 29, 2021

This comment has been minimized.

sfackler commented Mar 30, 2021

rfcbot commented Mar 30, 2021 •

edited by BurntSushi

Loading

rfcbot commented Mar 30, 2021

sfackler commented Mar 30, 2021

bors commented Mar 30, 2021

bors commented Mar 30, 2021

bors commented Mar 30, 2021

pthariensflame commented Apr 8, 2021

cuviper commented Apr 8, 2021

huangjj27 commented Aug 7, 2021 •

edited

Loading

Disallow octal format in Ipv4 string #83652

Disallow octal format in Ipv4 string #83652

Conversation

xu-cheng commented Mar 29, 2021

rust-highfive commented Mar 29, 2021

This comment has been minimized.

sfackler commented Mar 30, 2021

rfcbot commented Mar 30, 2021 • edited by BurntSushi Loading

rfcbot commented Mar 30, 2021

sfackler commented Mar 30, 2021

bors commented Mar 30, 2021

bors commented Mar 30, 2021

bors commented Mar 30, 2021

pthariensflame commented Apr 8, 2021

cuviper commented Apr 8, 2021

huangjj27 commented Aug 7, 2021 • edited Loading

rfcbot commented Mar 30, 2021 •

edited by BurntSushi

Loading

huangjj27 commented Aug 7, 2021 •

edited

Loading