From 1b94b84ad0143ea2039610e3aec9e929a8a20733 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 20 Sep 2018 09:31:14 -0700 Subject: [PATCH] std: Check for overflow in `str::repeat` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes a buffer overflow issue in the standard library discovered by Scott McMurray where if a large number was passed to `str::repeat` it may cause and out of bounds write to the buffer of a `Vec`. This bug was accidentally introduced in #48657 when optimizing the `str::repeat` function. The bug affects stable Rust releases 1.26.0 to 1.29.0. We plan on backporting this fix to create a 1.29.1 release, and the 1.30.0 release onwards will include this fix. The fix in this commit is to introduce a deterministic panic in the case of capacity overflow. When repeating a slice where the resulting length is larger than the address space, there’s no way it can succeed anyway! The standard library and surrounding libraries were briefly checked to see if there were othere instances of preallocating a vector with a calculation that may overflow. No instances of this bug (out of bounds write due to a calculation overflow) were found at this time. Note that this commit is the first steps towards fixing this issue, we'll be making a formal post to the Rust security list once these commits have been merged. --- RELEASES.md | 13 +++++++++++++ src/bootstrap/channel.rs | 2 +- src/liballoc/slice.rs | 16 +++++++++++++++- src/liballoc/str.rs | 13 +++++++++++++ 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 82472df727a07..6492f725318d0 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,3 +1,16 @@ +Version 1.29.1 (2018-09-25) +=========================== + +Security Notes +-------------- + +- The standard library's `str::repeat` function contained an out of bounds write + caused by an integer overflow. This has been fixed by deterministically + panicking when an overflow happens. + + Thank you to Scott McMurray for responsibily disclosing this vulnerability to + us. + Version 1.29.0 (2018-09-13) ========================== diff --git a/src/bootstrap/channel.rs b/src/bootstrap/channel.rs index 04d576df95546..e9540d6376ad9 100644 --- a/src/bootstrap/channel.rs +++ b/src/bootstrap/channel.rs @@ -24,7 +24,7 @@ use Build; use config::Config; // The version number -pub const CFG_RELEASE_NUM: &str = "1.29.0"; +pub const CFG_RELEASE_NUM: &str = "1.29.1"; pub struct GitInfo { inner: Option, diff --git a/src/liballoc/slice.rs b/src/liballoc/slice.rs index c27c596e7975a..73a48e6544957 100644 --- a/src/liballoc/slice.rs +++ b/src/liballoc/slice.rs @@ -392,6 +392,10 @@ impl [T] { /// Creates a vector by repeating a slice `n` times. /// + /// # Panics + /// + /// This function will panic if the capacity would overflow. + /// /// # Examples /// /// Basic usage: @@ -403,6 +407,16 @@ impl [T] { /// assert_eq!([1, 2].repeat(3), vec![1, 2, 1, 2, 1, 2]); /// } /// ``` + /// + /// A panic upon overflow: + /// + /// ```should_panic + /// #![feature(repeat_generic_slice)] + /// fn main() { + /// // this will panic at runtime + /// b"0123456789abcdef".repeat(usize::max_value()); + /// } + /// ``` #[unstable(feature = "repeat_generic_slice", reason = "it's on str, why not on slice?", issue = "48784")] @@ -417,7 +431,7 @@ impl [T] { // and `rem` is the remaining part of `n`. // Using `Vec` to access `set_len()`. - let mut buf = Vec::with_capacity(self.len() * n); + let mut buf = Vec::with_capacity(self.len().checked_mul(n).expect("capacity overflow")); // `2^expn` repetition is done by doubling `buf` `expn`-times. buf.extend(self); diff --git a/src/liballoc/str.rs b/src/liballoc/str.rs index 870bf971cd3f6..f1bc9a8398861 100644 --- a/src/liballoc/str.rs +++ b/src/liballoc/str.rs @@ -515,6 +515,10 @@ impl str { /// Create a [`String`] by repeating a string `n` times. /// + /// # Panics + /// + /// This function will panic if the capacity would overflow. + /// /// [`String`]: string/struct.String.html /// /// # Examples @@ -524,6 +528,15 @@ impl str { /// ``` /// assert_eq!("abc".repeat(4), String::from("abcabcabcabc")); /// ``` + /// + /// A panic upon overflow: + /// + /// ```should_panic + /// fn main() { + /// // this will panic at runtime + /// "0123456789abcdef".repeat(usize::max_value()); + /// } + /// ``` #[stable(feature = "repeat_str", since = "1.16.0")] pub fn repeat(&self, n: usize) -> String { unsafe { String::from_utf8_unchecked(self.as_bytes().repeat(n)) }