From 46ee9dd56ef5ba0e0a720568a0b4cbcfc3efa1a0 Mon Sep 17 00:00:00 2001 From: Serial <69764315+Serial-ATA@users.noreply.github.com> Date: Tue, 19 Nov 2024 23:24:39 -0500 Subject: [PATCH] WV: Stop manually indexing in block parsing --- CHANGELOG.md | 4 +- lofty/src/wavpack/properties.rs | 58 ++++++------------ ...cf46fbf0ef1285c4d84fbd39a919ef2b_minimized | Bin 0 -> 34 bytes lofty/tests/fuzz/wavpackfile_read_from.rs | 8 +++ 4 files changed, 28 insertions(+), 42 deletions(-) create mode 100644 lofty/tests/fuzz/assets/wavpackfile_read_from/crash-96407368cf46fbf0ef1285c4d84fbd39a919ef2b_minimized diff --git a/CHANGELOG.md b/CHANGELOG.md index 78aad5fb..8e5a7eff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,10 +39,10 @@ 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)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/479)): +- **Fuzzing** (Thanks [@qarmin](https://github.com/qarmin)!) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/476)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/479)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/483)): - **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 wrongly sized blocks ([issue](https://github.com/Serial-ATA/lofty-rs/issues/472)) ([issue](https://github.com/Serial-ATA/lofty-rs/issues/480)) - **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)) - **ID3v2**: Fix panic when parsing non-ASCII `TDAT` and `TIME` frames in `TDRC` conversion ([issue](https://github.com/Serial-ATA/lofty-rs/issues/477)) diff --git a/lofty/src/wavpack/properties.rs b/lofty/src/wavpack/properties.rs index 9cdd13b2..82e0cc21 100644 --- a/lofty/src/wavpack/properties.rs +++ b/lofty/src/wavpack/properties.rs @@ -302,28 +302,19 @@ fn get_extended_meta_info( block_content: &[u8], properties: &mut WavPackProperties, ) -> Result<()> { - let mut index = 0; - let block_size = block_content.len(); - while index < block_size { - if block_size - index < 2 { + let reader = &mut &block_content[..]; + loop { + if reader.len() < 2 { break; } - let id = block_content[index]; - index += 1; - - let mut size = u32::from(block_content[index]) << 1; - index += 1; + let id = reader.read_u8()?; + let mut size = u32::from(reader.read_u8()?) << 1; let is_large = id & ID_FLAG_LARGE_SIZE > 0; if is_large { - if block_size - index < 2 { - break; - } - - size += u32::from(block_content[index]) << 9; - size += u32::from(block_content[index + 1]) << 17; - index += 2; + size += u32::from(reader.read_u8()?) << 9; + size += u32::from(reader.read_u8()?) << 17; } if size == 0 { @@ -334,19 +325,20 @@ fn get_extended_meta_info( size -= 1; } + if (size as usize) >= reader.len() { + err!(SizeMismatch); + } + match id & 0x3F { ID_NON_STANDARD_SAMPLE_RATE => { - properties.sample_rate = - (&mut &block_content[index..]).read_u24::()?; - index += 3; + properties.sample_rate = reader.read_u24::()?; }, ID_DSD => { if size <= 1 { decode_err!(@BAIL WavPack, "Encountered an invalid DSD block size"); } - let mut rate_multiplier = u32::from(block_content[index]); - index += 1; + let mut rate_multiplier = u32::from(reader.read_u8()?); if rate_multiplier > 30 { parse_mode_choice!( @@ -360,62 +352,47 @@ fn get_extended_meta_info( properties.sample_rate = properties.sample_rate.wrapping_mul(rate_multiplier); // Skip DSD mode - index += 1; + let _ = reader.read_u8()?; }, ID_MULTICHANNEL => { if size <= 1 { decode_err!(@BAIL WavPack, "Unable to extract channel information"); } - properties.channels = u16::from(block_content[index]); - index += 1; - - let reader = &mut &block_content[index..]; + properties.channels = u16::from(reader.read_u8()?); // size - (id length + channel length) let s = size - 2; 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); }, @@ -423,12 +400,13 @@ fn get_extended_meta_info( } }, _ => { - index += size as usize; + let (_, rem) = reader.split_at(size as usize); + *reader = rem; }, } if id & ID_FLAG_ODD_SIZE > 0 { - index += 1; + let _ = reader.read_u8()?; } } diff --git a/lofty/tests/fuzz/assets/wavpackfile_read_from/crash-96407368cf46fbf0ef1285c4d84fbd39a919ef2b_minimized b/lofty/tests/fuzz/assets/wavpackfile_read_from/crash-96407368cf46fbf0ef1285c4d84fbd39a919ef2b_minimized new file mode 100644 index 0000000000000000000000000000000000000000..ed410e246de3f0095bfe8697e99c390f30210dcf GIT binary patch literal 34 dcmXRfE6A2&U|{$U2EJi_VA?grIS|AE@c~dq6yg8? literal 0 HcmV?d00001 diff --git a/lofty/tests/fuzz/wavpackfile_read_from.rs b/lofty/tests/fuzz/wavpackfile_read_from.rs index a8526de8..c2b2376f 100644 --- a/lofty/tests/fuzz/wavpackfile_read_from.rs +++ b/lofty/tests/fuzz/wavpackfile_read_from.rs @@ -104,3 +104,11 @@ fn panic3() { ); let _ = WavPackFile::read_from(&mut reader, ParseOptions::default()); } + +#[test_log::test] +fn panic4() { + let mut reader = crate::get_reader( + "wavpackfile_read_from/crash-96407368cf46fbf0ef1285c4d84fbd39a919ef2b_minimized", + ); + let _ = WavPackFile::read_from(&mut reader, ParseOptions::default()); +}