From 4e800e23465902505f2ed17aeb9ea774e114a58d Mon Sep 17 00:00:00 2001 From: Colin Murphy Date: Fri, 20 Dec 2024 15:29:59 -0500 Subject: [PATCH 1/2] fix: MLLT deviation field overflow --- src/stream/frame/content.rs | 38 ++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/src/stream/frame/content.rs b/src/stream/frame/content.rs index fb8cfffd4..ac753fd61 100644 --- a/src/stream/frame/content.rs +++ b/src/stream/frame/content.rs @@ -891,7 +891,13 @@ impl<'a> Decoder<'a> { deviations[i] = u32::try_from(carry >> (64 - bits_us)).map_err(|_| { Error::new(ErrorKind::InvalidInput, "MLLT deviation field overflow") })?; - carry <<= bits_us; + carry = carry + .checked_shl( + bits_us.try_into().map_err(|_| { + Error::new(ErrorKind::InvalidInput, "MLLT shift overflow") + })?, + ) + .ok_or_else(|| Error::new(ErrorKind::InvalidInput, "MLLT carry overflow"))?; carry_bits -= bits_us; } let [deviate_bytes, deviate_millis] = deviations; @@ -1908,4 +1914,34 @@ mod tests { assert_eq!(e.description, "MLLT deviation field overflow"); } } + + #[test] + fn test_mllt_shift_overflow() { + // Create a payload with large deviation values that would cause an overflow + let payload = [ + 0xFF, 0x02, // frames_between_reference (65282) + 0x00, 0x00, 0x00, // bytes_between_reference (0) + 0xFF, 0xFF, 0x62, // millis_between_reference (16777058) + 0x40, // bits_for_bytes (64) + 0x01, // bits_for_millis (01) + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x80, 0x44, 0x37, 0x00, + 0x62, // carry (4), carry_bits(64) + ]; + + // Combine header and payload into a single data stream + let mut data = Vec::new(); + data.extend_from_slice(&payload); + + let mut reader = Cursor::new(data); + + // Attempt to decode the frame + let result = decode("MLLT", Version::Id3v23, &mut reader); + + // Ensure that the result is an error due to overflow + assert!(result.is_err()); + if let Err(e) = result { + assert!(matches!(e.kind, ErrorKind::InvalidInput)); + assert_eq!(e.description, "MLLT carry overflow"); + } + } } From 08b6420b8e4d623bc82c2dce620a053b6a549174 Mon Sep 17 00:00:00 2001 From: Colin Murphy Date: Mon, 23 Dec 2024 09:20:39 -0500 Subject: [PATCH 2/2] fix: MLLT carry subtraction overflow --- src/stream/frame/content.rs | 44 ++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/src/stream/frame/content.rs b/src/stream/frame/content.rs index ac753fd61..858112fd9 100644 --- a/src/stream/frame/content.rs +++ b/src/stream/frame/content.rs @@ -870,7 +870,10 @@ impl<'a> Decoder<'a> { .by_ref() .take((bits_for_bytes_us + bits_for_millis_us) / 8) { - carry |= u64::from(b) << (64 - carry_bits - 8); + carry |= u64::from(b) + << (64_usize.checked_sub(carry_bits + 8_usize).ok_or_else(|| { + Error::new(ErrorKind::InvalidInput, "MLLT carry subtraction overflow") + })?); carry_bits += 8; } // Shift 2 deviation fields from the carry accumulator. @@ -1944,4 +1947,43 @@ mod tests { assert_eq!(e.description, "MLLT carry overflow"); } } + + #[test] + fn test_mllt_subtract_overflow() { + // Create a payload with large deviation values that would cause an overflow + let payload = [ + 0xBC, 0xBC, // frames_between_reference (48316) + 0xBC, 0xBC, 0xBC, // bytes_between_reference (12369084) + 0xBC, 0xBC, 0xBC, // millis_between_reference (12369084) + 0xBC, // bits_for_bytes (118) + 0xBC, // bits_for_millis (118) + 0x62, 0x62, 0x63, 0x00, 0x3B, 0x00, 0x65, 0x62, 0x62, 0x7A, 0x62, 0x62, 0xFF, 0x42, + 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, + 0x42, 0x42, 0x42, 0x42, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xBC, 0xBC, 0xBC, 0xBC, 0xBC, 0xBC, 0xBC, 0x62, 0x62, 0x63, 0x00, 0x3B, 0x00, 0x65, + 0x62, 0x62, 0x7A, 0x62, 0x62, 0xFF, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, + 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x09, + 0x00, 0xFF, 0xC1, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x3F, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0x62, 0xFF, 0x39, 0x00, 0xD3, 0x00, 0x00, 0x00, 0x00, 0x00, + 0xEA, + ]; + + // Combine header and payload into a single data stream + let mut data = Vec::new(); + data.extend_from_slice(&payload); + + let mut reader = Cursor::new(data); + + // Attempt to decode the frame + let result = decode("MLLT", Version::Id3v23, &mut reader); + + // Ensure that the result is an error due to overflow + assert!(result.is_err()); + if let Err(e) = result { + assert!(matches!(e.kind, ErrorKind::InvalidInput)); + assert_eq!(e.description, "MLLT carry subtraction overflow"); + } + } }