Fix panic in the JER OctetString decoder if the string isn't even length (#382)

decryphe · web-flow · commit 6f708c40bcac · 2024-12-05T17:50:52.000+01:00
diff --git a/src/jer/de.rs b/src/jer/de.rs
@@ -70,7 +70,7 @@ impl crate::Decoder for Decoder {
                     self.codec(),
                 )
             })?;
-            (value, *size as u64)
+            (value, *size)
         } else {
             let last = self.stack.pop().ok_or_else(JerDecodeErrorKind::eoi)?;
             let value_map = last
@@ -86,20 +86,26 @@ impl crate::Decoder for Decoder {
                     value_map
                         .get("length")
                         .and_then(|l| l.as_number())
-                        .and_then(|i| i.as_u64()),
+                        .and_then(|i| i.as_u64())
+                        .map(|i| i as usize),
                 )
                 .ok_or_else(|| JerDecodeErrorKind::TypeMismatch {
                     needed: "JSON object containing 'value' and 'length' properties.",
                     found: alloc::format!("{value_map:#?}"),
                 })?;
-            (
-                (0..value.len())
-                    .step_by(2)
-                    .map(|i| u8::from_str_radix(&value[i..=i + 1], 16))
-                    .collect::<Result<BitString, _>>()
-                    .map_err(|e| JerDecodeErrorKind::InvalidJerBitstring { parse_int_err: e })?,
-                length,
-            )
+
+            let value = bytes_from_hexstring(value).ok_or(DecodeError::custom(
+                alloc::format!("Failed to create BitString from bytes: {value:02x?}"),
+                self.codec(),
+            ))?;
+            let value = BitString::try_from_vec(value).map_err(|e| {
+                DecodeError::custom(
+                    alloc::format!("Failed to create BitString from bytes: {e:02x?}"),
+                    self.codec(),
+                )
+            })?;
+
+            (value, length)
         };
         let padding_length = if bitstring_length % 8 == 0 {
             0
@@ -109,7 +115,15 @@ impl crate::Decoder for Decoder {
         for _ in 0..padding_length {
             padded.pop();
         }
-        Ok(padded)
+
+        if bitstring_length != padded.len() {
+            Err(DecodeError::custom(
+                alloc::format!("Failed to create BitString from bytes: invalid value length (was: {}, expected: {})", padded.len(), bitstring_length),
+                self.codec(),
+            ))
+        } else {
+            Ok(padded)
+        }
     }
 
     fn decode_bool(&mut self, _t: Tag) -> Result<bool, Self::Error> {
@@ -605,11 +619,8 @@ impl Decoder {
                 needed: "hex string",
                 found: alloc::format!("{value}"),
             })?;
-        Ok((0..octet_string.len())
-            .step_by(2)
-            .map(|i| u8::from_str_radix(&octet_string[i..=i + 1], 16))
-            .collect::<Result<alloc::vec::Vec<u8>, _>>()
-            .map_err(|_| JerDecodeErrorKind::InvalidJerOctetString {})?)
+        bytes_from_hexstring(octet_string)
+            .ok_or(JerDecodeErrorKind::InvalidJerOctetString {}.into())
     }
 
     fn utc_time_from_value(value: Value) -> Result<chrono::DateTime<chrono::Utc>, DecodeError> {
@@ -647,3 +658,100 @@ impl Decoder {
         })?)
     }
 }
+
+/// Parses a hex string into bytes.
+fn bytes_from_hexstring(hex_string: &str) -> Option<alloc::vec::Vec<u8>> {
+    if hex_string.len() % 2 != 0 {
+        return None;
+    }
+    let mut bytes = alloc::vec::Vec::<u8>::with_capacity(hex_string.len() / 2);
+    for (i, c) in hex_string.char_indices() {
+        let n = nibble_from_hexdigit(c)?;
+        if i % 2 == 0 {
+            bytes.push(n << 4);
+        } else {
+            bytes[i / 2] |= n;
+        }
+    }
+    Some(bytes)
+}
+
+/// Parses a hexdigit character into a nibble (four bits).
+fn nibble_from_hexdigit(c: char) -> Option<u8> {
+    match c {
+        '0'..='9' => Some(c as u8 - b'0'),
+        'a'..='f' => Some(c as u8 - b'a' + 0xA),
+        'A'..='F' => Some(c as u8 - b'A' + 0xA),
+        _ => None,
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_bytes_from_hexstring() {
+        assert_eq!(bytes_from_hexstring(""), Some(vec![]));
+        assert_eq!(bytes_from_hexstring("00"), Some(vec![0]));
+        assert_eq!(bytes_from_hexstring("FF"), Some(vec![0xFF]));
+        assert_eq!(bytes_from_hexstring("0000"), Some(vec![0, 0]));
+        assert_eq!(bytes_from_hexstring("FFFF"), Some(vec![0xFF, 0xFF]));
+
+        assert_eq!(bytes_from_hexstring(" "), None);
+        assert_eq!(bytes_from_hexstring("!"), None);
+        assert_eq!(bytes_from_hexstring("0"), None);
+        assert_eq!(bytes_from_hexstring(" 0"), None);
+        assert_eq!(bytes_from_hexstring("0 "), None);
+        assert_eq!(bytes_from_hexstring("0!"), None);
+        assert_eq!(bytes_from_hexstring("  "), None);
+        assert_eq!(bytes_from_hexstring("00 "), None);
+        assert_eq!(bytes_from_hexstring(" 00"), None);
+        assert_eq!(bytes_from_hexstring("000"), None);
+        assert_eq!(bytes_from_hexstring("Œ"), None);
+        assert_eq!(bytes_from_hexstring("ŒŒ"), None);
+        assert_eq!(bytes_from_hexstring("ŒŒŒ"), None);
+        assert_eq!(bytes_from_hexstring("ABCDEFG"), None);
+        assert_eq!(bytes_from_hexstring(" ABCDEF"), None);
+        assert_eq!(bytes_from_hexstring("\u{0000}"), None);
+        assert_eq!(bytes_from_hexstring("\u{FFFF}"), None);
+        assert_eq!(bytes_from_hexstring("\u{0123}"), None);
+        assert_eq!(bytes_from_hexstring("\u{30}"), None);
+        assert_eq!(bytes_from_hexstring("\\u0030"), None);
+        assert_eq!(bytes_from_hexstring("\\u202E\\u0030\\u0030"), None);
+        assert_eq!(bytes_from_hexstring("⣐⡄"), None);
+        assert_eq!(bytes_from_hexstring("😎"), None);
+        assert_eq!(bytes_from_hexstring("🙈🙉🙊"), None);
+    }
+
+    #[test]
+    fn test_nibble_from_hexdigit() {
+        for c in '\u{0}'..'\u{1024}' {
+            match c {
+                '0' => assert_eq!(Some(0x00), nibble_from_hexdigit(c)),
+                '1' => assert_eq!(Some(0x01), nibble_from_hexdigit(c)),
+                '2' => assert_eq!(Some(0x02), nibble_from_hexdigit(c)),
+                '3' => assert_eq!(Some(0x03), nibble_from_hexdigit(c)),
+                '4' => assert_eq!(Some(0x04), nibble_from_hexdigit(c)),
+                '5' => assert_eq!(Some(0x05), nibble_from_hexdigit(c)),
+                '6' => assert_eq!(Some(0x06), nibble_from_hexdigit(c)),
+                '7' => assert_eq!(Some(0x07), nibble_from_hexdigit(c)),
+                '8' => assert_eq!(Some(0x08), nibble_from_hexdigit(c)),
+                '9' => assert_eq!(Some(0x09), nibble_from_hexdigit(c)),
+                'A' => assert_eq!(Some(0x0A), nibble_from_hexdigit(c)),
+                'B' => assert_eq!(Some(0x0B), nibble_from_hexdigit(c)),
+                'C' => assert_eq!(Some(0x0C), nibble_from_hexdigit(c)),
+                'D' => assert_eq!(Some(0x0D), nibble_from_hexdigit(c)),
+                'E' => assert_eq!(Some(0x0E), nibble_from_hexdigit(c)),
+                'F' => assert_eq!(Some(0x0F), nibble_from_hexdigit(c)),
+                'a' => assert_eq!(Some(0x0A), nibble_from_hexdigit(c)),
+                'b' => assert_eq!(Some(0x0B), nibble_from_hexdigit(c)),
+                'c' => assert_eq!(Some(0x0C), nibble_from_hexdigit(c)),
+                'd' => assert_eq!(Some(0x0D), nibble_from_hexdigit(c)),
+                'e' => assert_eq!(Some(0x0E), nibble_from_hexdigit(c)),
+                'f' => assert_eq!(Some(0x0F), nibble_from_hexdigit(c)),
+                _ => assert_eq!(None, nibble_from_hexdigit(c)),
+            }
+        }
+    }
+}
diff --git a/tests/strings.rs b/tests/strings.rs
@@ -1,4 +1,3 @@
-// Test whether constrained OctetString and FixedOctetString are equal
 use bitvec::prelude::*;
 use rasn::prelude::*;
 use rasn::{ber, jer, oer, uper};
@@ -89,6 +88,7 @@ fn build_fixed_octet() -> ConstrainedHashes {
     }
 }
 
+// Test whether constrained OctetString and FixedOctetString are equal
 macro_rules! test_decode_eq {
     ($fn_name:ident, $codec:ident) => {
         #[test]
@@ -132,3 +132,198 @@ test_decode_eq!(test_uper_octet_eq, uper);
 test_decode_eq!(test_oer_octet_eq, oer);
 test_decode_eq!(test_ber_octet_eq, ber);
 test_decode_eq!(test_jer_octet_eq, jer);
+
+#[derive(AsnType, Decode, Encode, Debug, Clone, PartialEq)]
+#[rasn(automatic_tags)]
+pub struct ABitString {
+    #[rasn(size("0..=255"))]
+    pub the_string: BitString,
+}
+
+/// Tests that valid strings are parsed and invalid strings are rejected.
+#[test]
+fn test_jer_bitstring_dec() {
+    use bitvec::prelude::*;
+
+    let good_cases: Vec<(&str, usize, BitVec<u8, bitvec::order::Msb0>)> = vec![
+        ("", 0, bitvec::bits![u8, Msb0;].into()),
+        ("00", 1, bitvec::bits![u8, Msb0; 0].into()),
+        ("00", 3, bitvec::bits![u8, Msb0; 0,0,0].into()),
+        ("0F", 3, bitvec::bits![u8, Msb0; 0,0,0].into()),
+        ("F0", 3, bitvec::bits![u8, Msb0; 1,1,1].into()),
+        ("00", 7, bitvec::bits![u8, Msb0; 0,0,0,0,0,0,0].into()),
+        ("00", 8, bitvec::bits![u8, Msb0; 0,0,0,0,0,0,0,0].into()),
+        ("0F", 8, bitvec::bits![u8, Msb0; 0,0,0,0,1,1,1,1].into()),
+        (
+            "\\u0030\\u0030",
+            8,
+            bitvec::bits![u8, Msb0; 0,0,0,0,0,0,0,0].into(),
+        ),
+        (
+            "\\u0046\\u0046",
+            8,
+            bitvec::bits![u8, Msb0; 1,1,1,1,1,1,1,1].into(),
+        ),
+    ];
+
+    let bad_cases: Vec<(&str, usize)> = vec![
+        (" ", 0),
+        ("!", 0),
+        ("0", 0),
+        (" 0", 0),
+        ("0 ", 0),
+        ("0!", 0),
+        ("  ", 0),
+        ("00 ", 0),
+        (" 00", 0),
+        ("000", 0),
+        ("Œ", 0),
+        ("ŒŒ", 0),
+        ("ŒŒŒ", 0),
+        ("ABCDEFG", 0),
+        (" ABCDEF", 0),
+        ("\u{0000}", 0),
+        ("\u{FFFF}", 0),
+        ("\u{0123}", 0),
+        ("\u{30}", 0),
+        ("\\u0030", 0),
+        ("\\u202E\\u0030\\u0030", 0),
+        ("⣐⡄", 0),
+        ("😎", 0),
+        ("🙈🙉🙊", 0),
+        ("", 1),
+        ("", 8),
+        ("00", 0),
+        ("00", 10),
+        ("00", 16),
+        ("00", 16384),
+        ("0000", 0),
+        ("0000", 8),
+        ("0000", 17),
+    ];
+
+    for (case, length, bits) in good_cases {
+        let json = format!("{{\"the_string\":{{\"length\":{length},\"value\":\"{case}\"}}}}");
+        let expected = ABitString { the_string: bits };
+        let decoded = jer::decode::<ABitString>(&json);
+        if let Err(e) = decoded {
+            panic!("should have decoded case \"{case}\" fine: {e:?}");
+        }
+        assert_eq!(decoded.unwrap(), expected);
+    }
+
+    for (case, length) in bad_cases {
+        let json = format!("{{\"the_string\":{{\"length\":{length},\"value\":\"{case}\"}}}}");
+        let decoded = jer::decode::<ABitString>(&json);
+        if let Ok(decoded) = decoded {
+            panic!(
+                "should have rejected case \"{case}\", but decoded: {decoded:?} (length {})",
+                decoded.the_string.len()
+            );
+        }
+    }
+}
+
+#[derive(AsnType, Decode, Encode, Debug, Clone, PartialEq)]
+#[rasn(automatic_tags)]
+pub struct AnOctetString {
+    #[rasn(size("0..=255"))]
+    pub the_string: OctetString,
+}
+
+/// Tests that valid strings are parsed and invalid strings are rejected.
+#[test]
+fn test_jer_octetstring_dec() {
+    let good_cases: Vec<&[u8]> = vec![
+        &[],
+        &[0x00; 1],
+        &[0x00; 2],
+        &[0x0F; 1],
+        &[0xFF; 1],
+        &[0xFF; 2],
+        &[0x00; 10],
+        &[0x00; 100],
+        &[0x00; 200],
+        &[0x01, 0x23],
+        &[0xAB, 0xCD],
+        &[0xAB, 0xCD, 0xEF],
+        &[0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF],
+        &[0x00; 255],
+        &[0x0F; 255],
+        &[0xFF; 255],
+        &[0x00; 256],
+        &[0x0F; 256],
+        &[0xFF; 256],
+        &[0x00; 16384],
+        &[0x0F; 16384],
+        &[0xFF; 16384],
+    ];
+
+    let special_cases: Vec<(&str, &[u8])> =
+        vec![("\\u0030\\u0030", &[0x00]), ("\\u0046\\u0046", &[0xFF])];
+
+    let bad_cases = vec![
+        " ",
+        "!",
+        "0",
+        " 0",
+        "0 ",
+        "0!",
+        "  ",
+        "000",
+        "Œ",
+        "ŒŒ",
+        "ŒŒŒ",
+        "ABCDEFG",
+        " ABCDEF",
+        "\u{0000}",
+        "\u{FFFF}",
+        "\u{0123}",
+        "\u{30}",
+        "\\u0030",
+        "\\u202E\\u0030\\u0030",
+        "⣐⡄",
+        "😎",
+        "🙈🙉🙊",
+    ];
+
+    for case in good_cases {
+        let upper_hex = case
+            .iter()
+            .map(|b| format!("{b:02X}"))
+            .collect::<Vec<String>>()
+            .join("");
+        let json = format!("{{\"the_string\":\"{upper_hex}\"}}");
+        let expected = AnOctetString {
+            the_string: case.into(),
+        };
+        let decoded = jer::decode::<AnOctetString>(&json);
+        if let Err(e) = decoded {
+            panic!("should have decoded case \"{upper_hex}\" fine: {e:?}");
+        }
+        assert_eq!(decoded.unwrap(), expected);
+    }
+
+    for (case, expected) in special_cases {
+        let json = format!("{{\"the_string\":\"{case}\"}}");
+        let expected = AnOctetString {
+            the_string: expected.into(),
+        };
+        let decoded = jer::decode::<AnOctetString>(&json);
+        if let Err(e) = decoded {
+            panic!("should have decoded case \"{case}\" fine: {e:?}");
+        }
+        assert_eq!(decoded.unwrap(), expected);
+    }
+
+    for case in bad_cases {
+        let json = format!("{{\"the_string\":\"{case}\"}}");
+        let decoded = jer::decode::<AnOctetString>(&json);
+        if let Ok(decoded) = decoded {
+            panic!(
+                "should have rejected case \"{case}\", but decoded: {decoded:?} (length {})",
+                decoded.the_string.len()
+            );
+        }
+    }
+}
