From 6e6d439aa86e000f3b998257067454260d5ea955 Mon Sep 17 00:00:00 2001 From: Jay White Date: Fri, 26 Jan 2024 10:52:17 -0500 Subject: [PATCH 1/5] fix: decode now fails with u64::MAX + 1 --- src/varint.rs | 7 +++++-- src/varint_tests.rs | 6 ++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/varint.rs b/src/varint.rs index 7c48d7c..a7a299c 100644 --- a/src/varint.rs +++ b/src/varint.rs @@ -140,8 +140,11 @@ impl VarInt for u64 { result |= (msb_dropped as u64) << shift; shift += 7; - if b & MSB == 0 || shift > (9 * 7) { - success = b & MSB == 0; + if shift > (9 * 7) { + success = *b == 1; + break; + } else if b & MSB == 0 { + success = true; break; } } diff --git a/src/varint_tests.rs b/src/varint_tests.rs index c4a0928..bb5ea01 100644 --- a/src/varint_tests.rs +++ b/src/varint_tests.rs @@ -50,6 +50,12 @@ mod tests { ); } + #[test] + fn test_decode_max_u64_plus_one() { + let max_vec_encoded = vec![0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x02]; + assert!(u64::decode_var(max_vec_encoded.as_slice()).is_none()); + } + #[test] fn test_encode_i64() { assert_eq!((0 as i64).encode_var_vec(), (0 as u32).encode_var_vec()); From 2660f3388174ac33cb11f6317eac7a8e7dd5b510 Mon Sep 17 00:00:00 2001 From: Jay White Date: Fri, 26 Jan 2024 10:58:15 -0500 Subject: [PATCH 2/5] don't fail with optional leading 0 --- src/varint.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/varint.rs b/src/varint.rs index a7a299c..18b8c4b 100644 --- a/src/varint.rs +++ b/src/varint.rs @@ -141,7 +141,7 @@ impl VarInt for u64 { shift += 7; if shift > (9 * 7) { - success = *b == 1; + success = *b <= 1; break; } else if b & MSB == 0 { success = true; From 03a104cfbc506c54752ebacd4d8b364b9bf0f180 Mon Sep 17 00:00:00 2001 From: Jay White Date: Mon, 29 Jan 2024 08:19:17 -0500 Subject: [PATCH 3/5] fix: overflow in smaller types --- src/varint.rs | 17 ++++++++++++++--- src/varint_tests.rs | 5 +---- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/varint.rs b/src/varint.rs index 18b8c4b..bfc53bd 100644 --- a/src/varint.rs +++ b/src/varint.rs @@ -85,7 +85,12 @@ macro_rules! impl_varint { fn decode_var(src: &[u8]) -> Option<(Self, usize)> { let (n, s) = u64::decode_var(src)?; - Some((n as Self, s)) + // BUGIFX: this check is required to ensure that we actually return `None` when `src` has a value that would overflow `Self`. + if n > (Self::MAX as u64) { + None + } else { + Some((n as Self, s)) + } } fn encode_var(self, dst: &mut [u8]) -> usize { @@ -101,7 +106,12 @@ macro_rules! impl_varint { fn decode_var(src: &[u8]) -> Option<(Self, usize)> { let (n, s) = i64::decode_var(src)?; - Some((n as Self, s)) + // BUGIFX: this check is required to ensure that we actually return `None` when `src` has a value that would overflow `Self`. + if n > (Self::MAX as i64) || n < (Self::MIN as i64) { + None + } else { + Some((n as Self, s)) + } } fn encode_var(self, dst: &mut [u8]) -> usize { @@ -141,7 +151,8 @@ impl VarInt for u64 { shift += 7; if shift > (9 * 7) { - success = *b <= 1; + // BUGIFX: this check is required to ensure that we actually return `None` when `src` has a value that would overflow `u64`. + success = *b < 2; break; } else if b & MSB == 0 { success = true; diff --git a/src/varint_tests.rs b/src/varint_tests.rs index bb5ea01..ff25782 100644 --- a/src/varint_tests.rs +++ b/src/varint_tests.rs @@ -4,10 +4,7 @@ mod tests { use crate::reader::VarIntAsyncReader; #[cfg(any(feature = "tokio_async", feature = "futures_async"))] use crate::writer::VarIntAsyncWriter; - - use crate::reader::VarIntReader; - use crate::varint::VarInt; - use crate::writer::VarIntWriter; + use crate::{reader::VarIntReader, varint::VarInt, writer::VarIntWriter}; #[test] fn test_required_space() { From 3c30432216a969bebca58d1f5dd3eb059dc83fcc Mon Sep 17 00:00:00 2001 From: Jay White Date: Tue, 30 Jan 2024 13:50:26 -0500 Subject: [PATCH 4/5] revert smaller types change --- src/varint.rs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/varint.rs b/src/varint.rs index bfc53bd..875d865 100644 --- a/src/varint.rs +++ b/src/varint.rs @@ -85,12 +85,7 @@ macro_rules! impl_varint { fn decode_var(src: &[u8]) -> Option<(Self, usize)> { let (n, s) = u64::decode_var(src)?; - // BUGIFX: this check is required to ensure that we actually return `None` when `src` has a value that would overflow `Self`. - if n > (Self::MAX as u64) { - None - } else { - Some((n as Self, s)) - } + Some((n as Self, s)) } fn encode_var(self, dst: &mut [u8]) -> usize { @@ -106,12 +101,7 @@ macro_rules! impl_varint { fn decode_var(src: &[u8]) -> Option<(Self, usize)> { let (n, s) = i64::decode_var(src)?; - // BUGIFX: this check is required to ensure that we actually return `None` when `src` has a value that would overflow `Self`. - if n > (Self::MAX as i64) || n < (Self::MIN as i64) { - None - } else { - Some((n as Self, s)) - } + Some((n as Self, s)) } fn encode_var(self, dst: &mut [u8]) -> usize { From 5ef28b8533f6ac4c5da6b1360d7e31497132b94b Mon Sep 17 00:00:00 2001 From: Jay White Date: Tue, 30 Jan 2024 13:53:02 -0500 Subject: [PATCH 5/5] fmt --- src/varint_tests.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/varint_tests.rs b/src/varint_tests.rs index ff25782..bb5ea01 100644 --- a/src/varint_tests.rs +++ b/src/varint_tests.rs @@ -4,7 +4,10 @@ mod tests { use crate::reader::VarIntAsyncReader; #[cfg(any(feature = "tokio_async", feature = "futures_async"))] use crate::writer::VarIntAsyncWriter; - use crate::{reader::VarIntReader, varint::VarInt, writer::VarIntWriter}; + + use crate::reader::VarIntReader; + use crate::varint::VarInt; + use crate::writer::VarIntWriter; #[test] fn test_required_space() {