fix: prevent panics from malformed id3 metadata

* mllt overflow
* invalid frame id length
* decode content size underflow
* decode MLLT deviation overflow
* stream tag frame bytes underflow

Author: cdmurph32

src/stream/frame/content.rs

@@ -247,7 +247,9 @@ impl<W: io::Write> Encoder<W> {
         &mut self,
         content: &MpegLocationLookupTable,
     ) -> crate::Result<()> {
-        let ref_packed_size = content.bits_for_bytes + content.bits_for_millis;
+        let ref_packed_size = content
+            .bits_for_bytes
+            .saturating_add(content.bits_for_millis);
         if ref_packed_size % 4 != 0 {
             return Err(Error::new(
                 ErrorKind::InvalidInput,
@@ -886,7 +888,9 @@ impl<'a> Decoder<'a> {
                         ),
                     ));
                 }
-                deviations[i] = u32::try_from(carry >> (64 - bits_us)).unwrap();
+                deviations[i] = u32::try_from(carry >> (64 - bits_us)).map_err(|_| {
+                    Error::new(ErrorKind::InvalidInput, "MLLT deviation field overflow")
+                })?;
                 carry <<= bits_us;
                 carry_bits -= bits_us;
             }
@@ -1039,6 +1043,7 @@ mod tests {
     use crate::frame::Content;
     use crate::frame::{self, Picture, PictureType};
     use std::collections::HashMap;
+    use std::io::Cursor;
 
     fn bytes_for_encoding(text: &str, encoding: Encoding) -> Vec<u8> {
         encoding.encode(text)
@@ -1835,4 +1840,72 @@ mod tests {
         )
         .is_none());
     }
+
+    #[test]
+    fn test_encode_mllt_overflow() {
+        let mllt = Content::MpegLocationLookupTable(MpegLocationLookupTable {
+            frames_between_reference: 1,
+            bytes_between_reference: 418,
+            millis_between_reference: 15,
+            bits_for_bytes: 252,
+            bits_for_millis: 252, // This would cause an overflow if not for saturating_add
+            references: vec![
+                MpegLocationLookupTableReference {
+                    deviate_bytes: 0x1,
+                    deviate_millis: 0x2,
+                },
+                MpegLocationLookupTableReference {
+                    deviate_bytes: 0x3,
+                    deviate_millis: 0x4,
+                },
+                MpegLocationLookupTableReference {
+                    deviate_bytes: 0x5,
+                    deviate_millis: 0x6,
+                },
+            ],
+        });
+
+        let mut data_out = Vec::new();
+        let result = encode(&mut data_out, &mllt, Version::Id3v23, Encoding::UTF8);
+
+        // Error since 255 is not a multiple of 4, but no panic.
+        assert!(result.is_err());
+        if let Err(e) = result {
+            assert!(matches!(e.kind, ErrorKind::InvalidInput));
+            assert_eq!(
+                e.description,
+                "MLLT bits_for_bytes + bits_for_millis must be a multiple of 4"
+            );
+        }
+    }
+
+    #[test]
+    fn test_decode_mllt_deviation_overflow() {
+        // Create a payload with large deviation values that would overflow u32
+        let payload = [
+            0xFF, 0xFF, // frames_between_reference (u16::MAX)
+            0xFF, 0xFF, 0xFF, // bytes_between_reference (u24::MAX)
+            0xFF, 0xFF, 0xFF, // millis_between_reference (u24::MAX)
+            0x38, // bits_for_bytes (56)
+            0x1C, // bits_for_millis (28)
+            0xFF, 0xFF, 0xFF, 0xFF, // deviate_bytes (u32::MAX)
+            0xFF, 0xFF, 0xFF, 0xFF, // deviate_millis (u32::MAX)
+        ];
+
+        // 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 deviation field overflow");
+        }
+    }
 }

src/stream/frame/v4.rs

@@ -45,7 +45,7 @@ pub fn decode(mut reader: impl io::Read) -> crate::Result<Option<(usize, Frame)>
 
     let read_size = if flags.contains(Flags::DATA_LENGTH_INDICATOR) {
         let _decompressed_size = unsynch::decode_u32(reader.read_u32::<BigEndian>()?);
-        content_size - 4
+        content_size.saturating_sub(4)
     } else {
         content_size
     };
@@ -94,7 +94,12 @@ pub fn encode(mut writer: impl io::Write, frame: &Frame, flags: Flags) -> crate:
 
     writer.write_all({
         let id = frame.id().as_bytes();
-        assert_eq!(4, id.len());
+        if id.len() != 4 {
+            return Err(Error::new(
+                ErrorKind::InvalidInput,
+                "Frame ID must be 4 bytes long",
+            ));
+        }
         id
     })?;
     writer.write_u32::<BigEndian>(unsynch::encode_u32(
@@ -109,3 +114,51 @@ pub fn encode(mut writer: impl io::Write, frame: &Frame, flags: Flags) -> crate:
     writer.write_all(&content_buf)?;
     Ok(10 + comp_hint_delta + content_buf.len())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::frame::Content;
+    use std::io::Cursor;
+
+    #[test]
+    fn test_encode_with_invalid_frame_id() {
+        let frame = Frame::with_content("TST", Content::Text("Test content".to_string()));
+        let flags = Flags::empty();
+        let mut writer = Cursor::new(Vec::new());
+
+        let result = encode(&mut writer, &frame, flags);
+
+        assert!(result.is_err());
+        if let Err(e) = result {
+            assert!(matches!(e.kind, ErrorKind::InvalidInput));
+            assert_eq!(e.description, "Frame ID must be 4 bytes long");
+        }
+    }
+
+    #[test]
+    fn test_decode_with_underflow() {
+        // Create a frame header with DATA_LENGTH_INDICATOR flag set and a content size of 3
+        let frame_header = [
+            b'T', b'E', b'S', b'T', // Frame ID
+            0x00, 0x00, 0x00, 0x03, // Content size (3 bytes)
+            0x00, 0x01, // Flags (DATA_LENGTH_INDICATOR)
+        ];
+        // Create a reader with the frame header followed by 4 bytes for the decompressed size
+        let mut data = Vec::new();
+        data.extend_from_slice(&frame_header);
+        data.extend_from_slice(&[0x00, 0x00, 0x00, 0x04]); // Decompressed size (4 bytes)
+
+        let mut reader = Cursor::new(data);
+
+        // Attempt to decode the frame
+        let result = decode(&mut reader);
+
+        // Ensure that the result is an error due to underflow
+        assert!(result.is_err());
+        if let Err(e) = result {
+            assert!(matches!(e.kind, ErrorKind::Parsing));
+            assert_eq!(e.description, "Insufficient data to decode bytes");
+        }
+    }
+}

src/stream/tag.rs

@@ -65,7 +65,7 @@ impl Header {
     }
 
     fn frame_bytes(&self) -> u64 {
-        u64::from(self.tag_size) - u64::from(self.ext_header_size)
+        u64::from(self.tag_size).saturating_sub(u64::from(self.ext_header_size))
     }
 
     fn tag_size(&self) -> u64 {
@@ -1091,4 +1091,17 @@ mod tests {
 
         assert_eq!(tag, tag_read);
     }
+
+    #[test]
+    fn test_frame_bytes_underflow() {
+        let header = Header {
+            version: Version::Id3v24,
+            flags: Flags::empty(),
+            tag_size: 10,
+            ext_header_size: 20,
+        };
+
+        // Without saturating_sub, this would underflow and cause a panic.
+        assert_eq!(header.frame_bytes(), 0);
+    }
 }