~tdeo/serde_bare

6695663b5b4935d98abbca1cd35637a58101efbc — Richard Bradfield 11 months ago 5681bc1
Improve variable-length integer robustness

Move the maximum length check above the left shift, so we don't try and
shift more than 64 bits into the accumulator.

Return serde::de::Errors from the invalid branches of the Uint
deserialization implementation rather than panicking on bogus or
truncated input.

Also clean up a Clippy lint in the Uint serialize implementation, using
the .take(i) iterator adapter rather than a ranged for-loop.
1 files changed, 44 insertions(+), 15 deletions(-)

M src/lib.rs
M src/lib.rs => src/lib.rs +44 -15
@@ 57,8 57,8 @@ impl serde::ser::Serialize for Uint {
        i += 1;

        let mut s = serializer.serialize_tuple(usize::MAX)?;
        for j in 0..i {
            s.serialize_element(&buf[j])?;
        for b in buf.iter().take(i) {
            s.serialize_element(&b)?;
        }
        s.end()
    }


@@ 75,8 75,8 @@ impl<'de> serde::de::Deserialize<'de> for Uint {
        impl<'de> serde::de::Visitor<'de> for UintVisitor {
            type Value = Uint;

            fn expecting(&self, _formatter: &mut fmt::Formatter) -> fmt::Result {
                unreachable!()
            fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
                write!(formatter, "a BARE encoded variable-length integer")
            }

            fn visit_seq<A>(self, mut seq: A) -> std::result::Result<Self::Value, A::Error>


@@ 86,16 86,28 @@ impl<'de> serde::de::Deserialize<'de> for Uint {
                let mut x = 0u64;
                let mut s = 0usize;
                for i in 0.. {
                    // SeqAccess::next_element should never return None
                    let b = seq.next_element::<u8>()?.unwrap();
                    if b < 0x80 {
                    let b = seq.next_element::<u8>()?;
                    if let Some(b) = b {
                        if i > 9 || i == 9 && b > 1 {
                            todo!("error out");
                            // No more than 10 bytes can be in a BARE uint/int
                            return Err(serde::de::Error::custom(
                                "continuation bit indicated an invalid variable-length integer",
                            ));
                        }
                        if b < 0x80 {
                            // No continuation bit is set
                            return Ok(Uint(x | (b as u64) << s));
                        }
                        return Ok(Uint(x | (b as u64) << s));
                        x |= ((b & 0x7f) as u64) << s;
                        s += 7;
                    } else {
                        // Since we're calling next_element for u8 it's probably impossible to
                        // enter this branch without having raised an io::Error earlier, but better
                        // to handle it anyway instead of introducing a potential panic
                        return Err(serde::de::Error::custom(
                            "expected further bytes in variable-length integer",
                        ));
                    }
                    x |= ((b & 0x7f) as u64) << s;
                    s += 7;
                }
                unreachable!()
            }


@@ 169,10 181,7 @@ mod test {
            (0, &[0]),
            (1, &[1]),
            (275, &[147, 2]),
            (
                18446744073709551615,
                &[255, 255, 255, 255, 255, 255, 255, 255, 255, 1],
            ),
            (u64::MAX, &[255, 255, 255, 255, 255, 255, 255, 255, 255, 1]),
        ];
        for &(n, bytes) in CASES {
            println!("testing {}", n);


@@ 183,4 192,24 @@ mod test {
            assert_eq!(got_int, int);
        }
    }

    #[test]
    fn test_uint_too_long() {
        // Too many bytes
        let bytes: &'static [u8] = &[255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 1];
        let result = from_slice::<Uint>(&bytes);
        assert!(result.is_err());

        // Too many bits of precision (effectively u64::MAX + 1)
        let bytes: &'static [u8] = &[255, 255, 255, 255, 255, 255, 255, 255, 255, 2];
        let result = from_slice::<Uint>(&bytes);
        assert!(result.is_err());
    }

    #[test]
    fn test_uint_too_short() {
        let bytes: &'static [u8] = &[255, 255, 255];
        let result = from_slice::<Uint>(&bytes);
        assert!(result.is_err());
    }
}