From 24ead980daf11ba563e4fb2516187a56a71ad319 Mon Sep 17 00:00:00 2001 From: Alice Maz Date: Wed, 3 May 2017 05:26:40 +0000 Subject: [PATCH] Use checked arithmetic in encoded_size previously encoded_size could silently overflow usize, resulting in write past the bounds of the buffer allocated by reserve. this changes encoded_size to return an option, with none if overflow occurs. presently callers simply panic on this case, but it could conceivably be rendered as an error in the future credit to Andrew Ayer for reporting this vulnerability --- src/lib.rs | 136 +++++++++++++++++++++++++++++------------------------ 1 file changed, 75 insertions(+), 61 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index df424f11..3a1ade63 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -171,7 +171,10 @@ pub fn decode>(input: &T) -> Result, DecodeError ///} ///``` pub fn encode_config>(input: &T, config: Config) -> String { - let mut buf = String::with_capacity(encoded_size(input.as_ref().len(), config)); + let mut buf = match encoded_size(input.as_ref().len(), config) { + Some(n) => String::with_capacity(n), + None => panic!("integer overflow when calculating buffer size") + }; encode_config_buf(input, config, &mut buf); @@ -179,23 +182,25 @@ pub fn encode_config>(input: &T, config: Config) -> Stri } /// calculate the base64 encoded string size, including padding -fn encoded_size(bytes_len: usize, config: Config) -> usize { - let rem = bytes_len % 3; - - let complete_input_chunks = bytes_len / 3; - let complete_output_chars = complete_input_chunks * 4; - let printing_output_chars = if rem == 0 { - complete_output_chars - } else { - complete_output_chars + 4 - }; +fn encoded_size(bytes_len: usize, config: Config) -> Option { + let printing_output_chars = bytes_len + .checked_add(2) + .map(|x| x / 3) + .and_then(|x| x.checked_mul(4)); + + //TODO this is subtly wrong but in a not dangerous way + //pushing patch with identical to previous behavior, then fixing let line_ending_output_chars = match config.line_wrap { - LineWrap::NoWrap => 0, - LineWrap::Wrap(n, LineEnding::CRLF) => printing_output_chars / n * 2, - LineWrap::Wrap(n, LineEnding::LF) => printing_output_chars / n, + LineWrap::NoWrap => Some(0), + LineWrap::Wrap(n, LineEnding::CRLF) => + printing_output_chars.map(|y| y / n).and_then(|y| y.checked_mul(2)), + LineWrap::Wrap(n, LineEnding::LF) => + printing_output_chars.map(|y| y / n), }; - return printing_output_chars + line_ending_output_chars; + printing_output_chars.and_then(|x| + line_ending_output_chars.and_then(|y| x.checked_add(y)) + ) } ///Encode arbitrary octets as base64. @@ -224,7 +229,11 @@ pub fn encode_config_buf>(input: &T, config: Config, buf }; // reserve to make sure the memory we'll be writing to with unsafe is allocated - buf.reserve(encoded_size(input_bytes.len(), config)); + let resv_size = match encoded_size(input_bytes.len(), config) { + Some(n) => n, + None => panic!("integer overflow when calculating buffer size"), + }; + buf.reserve(resv_size); let orig_buf_len = buf.len(); let mut fast_loop_output_buf_len = orig_buf_len; @@ -579,52 +588,52 @@ mod tests { #[test] fn encoded_size_correct() { - assert_eq!(0, encoded_size(0, STANDARD)); + assert_eq!(Some(0), encoded_size(0, STANDARD)); - assert_eq!(4, encoded_size(1, STANDARD)); - assert_eq!(4, encoded_size(2, STANDARD)); - assert_eq!(4, encoded_size(3, STANDARD)); + assert_eq!(Some(4), encoded_size(1, STANDARD)); + assert_eq!(Some(4), encoded_size(2, STANDARD)); + assert_eq!(Some(4), encoded_size(3, STANDARD)); - assert_eq!(8, encoded_size(4, STANDARD)); - assert_eq!(8, encoded_size(5, STANDARD)); - assert_eq!(8, encoded_size(6, STANDARD)); + assert_eq!(Some(8), encoded_size(4, STANDARD)); + assert_eq!(Some(8), encoded_size(5, STANDARD)); + assert_eq!(Some(8), encoded_size(6, STANDARD)); - assert_eq!(12, encoded_size(7, STANDARD)); - assert_eq!(12, encoded_size(8, STANDARD)); - assert_eq!(12, encoded_size(9, STANDARD)); + assert_eq!(Some(12), encoded_size(7, STANDARD)); + assert_eq!(Some(12), encoded_size(8, STANDARD)); + assert_eq!(Some(12), encoded_size(9, STANDARD)); - assert_eq!(72, encoded_size(54, STANDARD)); + assert_eq!(Some(72), encoded_size(54, STANDARD)); - assert_eq!(76, encoded_size(55, STANDARD)); - assert_eq!(76, encoded_size(56, STANDARD)); - assert_eq!(76, encoded_size(57, STANDARD)); + assert_eq!(Some(76), encoded_size(55, STANDARD)); + assert_eq!(Some(76), encoded_size(56, STANDARD)); + assert_eq!(Some(76), encoded_size(57, STANDARD)); - assert_eq!(80, encoded_size(58, STANDARD)); + assert_eq!(Some(80), encoded_size(58, STANDARD)); } #[test] fn encoded_size_correct_mime() { - assert_eq!(0, encoded_size(0, MIME)); + assert_eq!(Some(0), encoded_size(0, MIME)); - assert_eq!(4, encoded_size(1, MIME)); - assert_eq!(4, encoded_size(2, MIME)); - assert_eq!(4, encoded_size(3, MIME)); + assert_eq!(Some(4), encoded_size(1, MIME)); + assert_eq!(Some(4), encoded_size(2, MIME)); + assert_eq!(Some(4), encoded_size(3, MIME)); - assert_eq!(8, encoded_size(4, MIME)); - assert_eq!(8, encoded_size(5, MIME)); - assert_eq!(8, encoded_size(6, MIME)); + assert_eq!(Some(8), encoded_size(4, MIME)); + assert_eq!(Some(8), encoded_size(5, MIME)); + assert_eq!(Some(8), encoded_size(6, MIME)); - assert_eq!(12, encoded_size(7, MIME)); - assert_eq!(12, encoded_size(8, MIME)); - assert_eq!(12, encoded_size(9, MIME)); + assert_eq!(Some(12), encoded_size(7, MIME)); + assert_eq!(Some(12), encoded_size(8, MIME)); + assert_eq!(Some(12), encoded_size(9, MIME)); - assert_eq!(72, encoded_size(54, MIME)); + assert_eq!(Some(72), encoded_size(54, MIME)); - assert_eq!(78, encoded_size(55, MIME)); - assert_eq!(78, encoded_size(56, MIME)); - assert_eq!(78, encoded_size(57, MIME)); + assert_eq!(Some(78), encoded_size(55, MIME)); + assert_eq!(Some(78), encoded_size(56, MIME)); + assert_eq!(Some(78), encoded_size(57, MIME)); - assert_eq!(82, encoded_size(58, MIME)); + assert_eq!(Some(82), encoded_size(58, MIME)); } #[test] @@ -636,26 +645,31 @@ mod tests { LineWrap::Wrap(76, LineEnding::LF) ); - assert_eq!(0, encoded_size(0, config)); + assert_eq!(Some(0), encoded_size(0, config)); + + assert_eq!(Some(4), encoded_size(1, config)); + assert_eq!(Some(4), encoded_size(2, config)); + assert_eq!(Some(4), encoded_size(3, config)); - assert_eq!(4, encoded_size(1, config)); - assert_eq!(4, encoded_size(2, config)); - assert_eq!(4, encoded_size(3, config)); + assert_eq!(Some(8), encoded_size(4, config)); + assert_eq!(Some(8), encoded_size(5, config)); + assert_eq!(Some(8), encoded_size(6, config)); - assert_eq!(8, encoded_size(4, config)); - assert_eq!(8, encoded_size(5, config)); - assert_eq!(8, encoded_size(6, config)); + assert_eq!(Some(12), encoded_size(7, config)); + assert_eq!(Some(12), encoded_size(8, config)); + assert_eq!(Some(12), encoded_size(9, config)); - assert_eq!(12, encoded_size(7, config)); - assert_eq!(12, encoded_size(8, config)); - assert_eq!(12, encoded_size(9, config)); + assert_eq!(Some(72), encoded_size(54, config)); - assert_eq!(72, encoded_size(54, config)); + assert_eq!(Some(77), encoded_size(55, config)); + assert_eq!(Some(77), encoded_size(56, config)); + assert_eq!(Some(77), encoded_size(57, config)); - assert_eq!(77, encoded_size(55, config)); - assert_eq!(77, encoded_size(56, config)); - assert_eq!(77, encoded_size(57, config)); + assert_eq!(Some(81), encoded_size(58, config)); + } - assert_eq!(81, encoded_size(58, config)); + #[test] + fn encoded_size_overflow() { + assert_eq!(None, encoded_size(std::usize::MAX, STANDARD)); } }