diff --git a/CHANGELOG.md b/CHANGELOG.md index 5191f9ee4..9ee81c0ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - When skipping invalid frames in `ParsingMode::{BestAttempt, Relaxed}`, the parser will no longer be able to go out of the bounds of the frame content ([issue](https://github.com/Serial-ATA/lofty-rs/issues/458)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/459)) - **MP4**: Support for flag items (ex. `cpil`) of any size (not just 1 byte) ([issue](https://github.com/Serial-ATA/lofty-rs/issues/457)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/460)) +- **Fuzzing** (Thanks [@qarmin](https://github.com/qarmin)!) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/476)): + - **MusePack**: Fix panic when ID3v2 tag sizes exceed the stream length ([issue](https://github.com/Serial-ATA/lofty-rs/issues/470)) + - **WAV**: Fix panic when calculating bit depth with abnormally large `bytes_per_sample` ([issue](https://github.com/Serial-ATA/lofty-rs/issues/471)) + - **WavPack***: Fix panic when encountering wrongly sized blocks ([issue](https://github.com/Serial-ATA/lofty-rs/issues/472)) + - **WavPack***: Fix panic when encountering zero-sized blocks ([issue](https://github.com/Serial-ATA/lofty-rs/issues/473)) + - **MPEG**: Fix panic when APE tags are incorrectly sized ([issue](https://github.com/Serial-ATA/lofty-rs/issues/474)) ## [0.21.1] - 2024-08-28 diff --git a/lofty/src/ape/read.rs b/lofty/src/ape/read.rs index cfd4807ec..675cace47 100644 --- a/lofty/src/ape/read.rs +++ b/lofty/src/ape/read.rs @@ -8,7 +8,7 @@ use crate::id3::v1::tag::Id3v1Tag; use crate::id3::v2::read::parse_id3v2; use crate::id3::v2::tag::Id3v2Tag; use crate::id3::{find_id3v1, find_id3v2, find_lyrics3v2, FindId3v2Config, ID3FindResults}; -use crate::macros::decode_err; +use crate::macros::{decode_err, err}; use std::io::{Read, Seek, SeekFrom}; @@ -37,12 +37,12 @@ where if let ID3FindResults(Some(header), content) = find_id3v2(data, find_id3v2_config)? { log::warn!("Encountered an ID3v2 tag. This tag cannot be rewritten to the APE file!"); - stream_len -= u64::from(header.size); + let Some(new_stream_length) = stream_len.checked_sub(u64::from(header.full_tag_size())) + else { + err!(SizeMismatch); + }; - // Exclude the footer - if header.flags.footer { - stream_len -= 10; - } + stream_len = new_stream_length; if let Some(content) = content { let reader = &mut &*content; @@ -107,16 +107,21 @@ where let ID3FindResults(id3v1_header, id3v1) = find_id3v1(data, parse_options.read_tags)?; if id3v1_header.is_some() { - stream_len -= 128; id3v1_tag = id3v1; + let Some(new_stream_length) = stream_len.checked_sub(128) else { + err!(SizeMismatch); + }; + + stream_len = new_stream_length; } // Next, check for a Lyrics3v2 tag, and skip over it, as it's no use to us - let ID3FindResults(lyrics3_header, lyrics3v2_size) = find_lyrics3v2(data)?; + let ID3FindResults(_, lyrics3v2_size) = find_lyrics3v2(data)?; + let Some(new_stream_length) = stream_len.checked_sub(u64::from(lyrics3v2_size)) else { + err!(SizeMismatch); + }; - if lyrics3_header.is_some() { - stream_len -= u64::from(lyrics3v2_size) - } + stream_len = new_stream_length; // Next, search for an APE tag footer // diff --git a/lofty/src/iff/wav/properties.rs b/lofty/src/iff/wav/properties.rs index 7788eb90a..e284af234 100644 --- a/lofty/src/iff/wav/properties.rs +++ b/lofty/src/iff/wav/properties.rs @@ -209,7 +209,7 @@ pub(super) fn read_properties( .. }) if valid_bits_per_sample > 0 => bit_depth = valid_bits_per_sample as u8, _ if bits_per_sample > 0 => bit_depth = bits_per_sample as u8, - _ => bit_depth = (bytes_per_sample * 8) as u8, + _ => bit_depth = bytes_per_sample.saturating_mul(8) as u8, }; let channel_mask = extensible_info.map(|info| info.channel_mask); diff --git a/lofty/src/mpeg/read.rs b/lofty/src/mpeg/read.rs index 77312f067..3fb3d5c10 100644 --- a/lofty/src/mpeg/read.rs +++ b/lofty/src/mpeg/read.rs @@ -163,7 +163,11 @@ where // Seek back to the start of the tag let pos = reader.stream_position()?; - reader.seek(SeekFrom::Start(pos - u64::from(header.size)))?; + let Some(start_of_tag) = pos.checked_sub(u64::from(header.size)) else { + err!(SizeMismatch); + }; + + reader.seek(SeekFrom::Start(start_of_tag))?; }, _ => { // Correct the position (APE header - Preamble) diff --git a/lofty/src/musepack/read.rs b/lofty/src/musepack/read.rs index 510b71b41..1d03ce939 100644 --- a/lofty/src/musepack/read.rs +++ b/lofty/src/musepack/read.rs @@ -32,12 +32,17 @@ where // ID3v2 tags are unsupported in MPC files, but still possible #[allow(unused_variables)] if let ID3FindResults(Some(header), Some(content)) = find_id3v2(reader, find_id3v2_config)? { + let Some(new_stream_length) = stream_length.checked_sub(u64::from(header.full_tag_size())) + else { + err!(SizeMismatch); + }; + + stream_length = new_stream_length; + let reader = &mut &*content; let id3v2 = parse_id3v2(reader, header, parse_options)?; file.id3v2_tag = Some(id3v2); - - stream_length -= u64::from(header.full_tag_size()); } // Save the current position, so we can go back and read the properties after the tags diff --git a/lofty/src/wavpack/properties.rs b/lofty/src/wavpack/properties.rs index b2b4857bf..f23639992 100644 --- a/lofty/src/wavpack/properties.rs +++ b/lofty/src/wavpack/properties.rs @@ -305,6 +305,10 @@ fn get_extended_meta_info( let mut index = 0; let block_size = block_content.len(); while index < block_size { + if block_size - index < 2 { + break; + } + let id = block_content[index]; index += 1; @@ -318,6 +322,10 @@ fn get_extended_meta_info( index += 2; } + if size == 0 { + decode_err!(@BAIL WavPack, "Encountered a zero-sized block"); + } + if id & ID_FLAG_ODD_SIZE > 0 { size -= 1; } @@ -326,6 +334,7 @@ fn get_extended_meta_info( ID_NON_STANDARD_SAMPLE_RATE => { properties.sample_rate = (&mut &block_content[index..]).read_u24::()?; + index += 3; }, ID_DSD => { if size <= 1 { @@ -364,32 +373,46 @@ fn get_extended_meta_info( match s { 0 => { let channel_mask = reader.read_u8()?; + index += 1; + properties.channel_mask = ChannelMask(u32::from(channel_mask)); }, 1 => { let channel_mask = reader.read_u16::()?; + index += 2; + properties.channel_mask = ChannelMask(u32::from(channel_mask)); }, 2 => { let channel_mask = reader.read_u24::()?; + index += 3; + properties.channel_mask = ChannelMask(channel_mask); }, 3 => { let channel_mask = reader.read_u32::()?; + index += 4; + properties.channel_mask = ChannelMask(channel_mask); }, 4 => { properties.channels |= u16::from(reader.read_u8()? & 0xF) << 8; properties.channels += 1; + index += 1; let channel_mask = reader.read_u24::()?; + index += 3; + properties.channel_mask = ChannelMask(channel_mask); }, 5 => { properties.channels |= u16::from(reader.read_u8()? & 0xF) << 8; properties.channels += 1; + index += 1; let channel_mask = reader.read_u32::()?; + index += 4; + properties.channel_mask = ChannelMask(channel_mask); }, _ => decode_err!(@BAIL WavPack, "Encountered invalid channel info size"), diff --git a/lofty/src/wavpack/read.rs b/lofty/src/wavpack/read.rs index 2656587a6..a12ef20b8 100644 --- a/lofty/src/wavpack/read.rs +++ b/lofty/src/wavpack/read.rs @@ -4,6 +4,7 @@ use crate::config::ParseOptions; use crate::error::Result; use crate::id3::{find_id3v1, find_lyrics3v2, ID3FindResults}; +use crate::macros::err; use std::io::{Read, Seek, SeekFrom}; pub(super) fn read_from(reader: &mut R, parse_options: ParseOptions) -> Result @@ -20,16 +21,21 @@ where let ID3FindResults(id3v1_header, id3v1) = find_id3v1(reader, parse_options.read_tags)?; if id3v1_header.is_some() { - stream_length -= 128; id3v1_tag = id3v1; + let Some(new_stream_length) = stream_length.checked_sub(128) else { + err!(SizeMismatch); + }; + + stream_length = new_stream_length; } // Next, check for a Lyrics3v2 tag, and skip over it, as it's no use to us - let ID3FindResults(lyrics3_header, lyrics3v2_size) = find_lyrics3v2(reader)?; + let ID3FindResults(_, lyrics3v2_size) = find_lyrics3v2(reader)?; + let Some(new_stream_length) = stream_length.checked_sub(u64::from(lyrics3v2_size)) else { + err!(SizeMismatch); + }; - if lyrics3_header.is_some() { - stream_length -= u64::from(lyrics3v2_size); - } + stream_length = new_stream_length; // Next, search for an APE tag footer // diff --git a/lofty/tests/fuzz/assets/mpcfile_read_from/crash-c98d99ebce28b50b50eb2e96320f02e1e729e543 b/lofty/tests/fuzz/assets/mpcfile_read_from/crash-c98d99ebce28b50b50eb2e96320f02e1e729e543 new file mode 100644 index 000000000..e8604ee40 Binary files /dev/null and b/lofty/tests/fuzz/assets/mpcfile_read_from/crash-c98d99ebce28b50b50eb2e96320f02e1e729e543 differ diff --git a/lofty/tests/fuzz/assets/mpegfile_read_from/crash-7b5eb183cc14faf3ecc93bdf4a5e30b05f7a537d_minimized b/lofty/tests/fuzz/assets/mpegfile_read_from/crash-7b5eb183cc14faf3ecc93bdf4a5e30b05f7a537d_minimized new file mode 100644 index 000000000..c4e457c0d Binary files /dev/null and b/lofty/tests/fuzz/assets/mpegfile_read_from/crash-7b5eb183cc14faf3ecc93bdf4a5e30b05f7a537d_minimized differ diff --git a/lofty/tests/fuzz/assets/wavfile_read_from/aa b/lofty/tests/fuzz/assets/wavfile_read_from/aa new file mode 100644 index 000000000..f0afb8ab8 Binary files /dev/null and b/lofty/tests/fuzz/assets/wavfile_read_from/aa differ diff --git a/lofty/tests/fuzz/assets/wavpackfile_read_from/bb b/lofty/tests/fuzz/assets/wavpackfile_read_from/bb new file mode 100644 index 000000000..01457ab64 Binary files /dev/null and b/lofty/tests/fuzz/assets/wavpackfile_read_from/bb differ diff --git a/lofty/tests/fuzz/assets/wavpackfile_read_from/output b/lofty/tests/fuzz/assets/wavpackfile_read_from/output new file mode 100644 index 000000000..a7cfeefcd Binary files /dev/null and b/lofty/tests/fuzz/assets/wavpackfile_read_from/output differ diff --git a/lofty/tests/fuzz/mpcfile_read_from.rs b/lofty/tests/fuzz/mpcfile_read_from.rs index 98acc2441..99cb6656e 100644 --- a/lofty/tests/fuzz/mpcfile_read_from.rs +++ b/lofty/tests/fuzz/mpcfile_read_from.rs @@ -8,3 +8,11 @@ fn panic1() { let mut reader = crate::get_reader("mpcfile_read_from/output.aac"); let _ = MpcFile::read_from(&mut reader, ParseOptions::new()); } + +// Overflow on badly sized ID3v2 tag +#[test_log::test] +fn panic2() { + let mut reader = + crate::get_reader("mpcfile_read_from/crash-c98d99ebce28b50b50eb2e96320f02e1e729e543"); + let _ = MpcFile::read_from(&mut reader, ParseOptions::new()); +} diff --git a/lofty/tests/fuzz/mpegfile_read_from.rs b/lofty/tests/fuzz/mpegfile_read_from.rs index 9c1df1aaa..ef47c036f 100644 --- a/lofty/tests/fuzz/mpegfile_read_from.rs +++ b/lofty/tests/fuzz/mpegfile_read_from.rs @@ -19,6 +19,13 @@ fn crash2() { let _ = MpegFile::read_from(&mut reader, ParseOptions::new()); } +#[test_log::test] +fn crash3() { + let mut reader = + get_reader("mpegfile_read_from/crash-7b5eb183cc14faf3ecc93bdf4a5e30b05f7a537d_minimized"); + let _ = MpegFile::read_from(&mut reader, ParseOptions::new()); +} + #[test_log::test] fn oom1() { oom_test::("mpegfile_read_from/oom-f8730cbfa5682ab12343ccb70de9b71a061ef4d0"); diff --git a/lofty/tests/fuzz/wavfile_read_from.rs b/lofty/tests/fuzz/wavfile_read_from.rs index 6da823485..f98f370ff 100644 --- a/lofty/tests/fuzz/wavfile_read_from.rs +++ b/lofty/tests/fuzz/wavfile_read_from.rs @@ -28,3 +28,9 @@ fn panic3() { crate::get_reader("wavfile_read_from/2_IDX_34_RAND_128635499166458268533001.wav"); let _ = WavFile::read_from(&mut reader, ParseOptions::new()); } + +#[test_log::test] +fn panic4() { + let mut reader = crate::get_reader("wavfile_read_from/aa"); + let _ = WavFile::read_from(&mut reader, ParseOptions::new()); +} diff --git a/lofty/tests/fuzz/wavpackfile_read_from.rs b/lofty/tests/fuzz/wavpackfile_read_from.rs index 7e827ab75..dca46f7fb 100644 --- a/lofty/tests/fuzz/wavpackfile_read_from.rs +++ b/lofty/tests/fuzz/wavpackfile_read_from.rs @@ -1,4 +1,6 @@ use crate::oom_test; +use lofty::config::ParseOptions; +use lofty::file::AudioFile; use lofty::wavpack::WavPackFile; #[test_log::test] @@ -82,3 +84,15 @@ fn oom11() { fn oom12() { oom_test::("wavpackfile_read_from/oom-94867b6fefcd32cd5bc3bc298468cd3d65d93ff1"); } + +#[test_log::test] +fn panic1() { + let mut reader = crate::get_reader("wavpackfile_read_from/output"); + let _ = WavPackFile::read_from(&mut reader, ParseOptions::default()); +} + +#[test_log::test] +fn panic2() { + let mut reader = crate::get_reader("wavpackfile_read_from/bb"); + let _ = WavPackFile::read_from(&mut reader, ParseOptions::default()); +}