From c47309dbd0a5988386a68378cd39766d78ee39b3 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 1/3] 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 78aad5fb1..8e5a7eff7 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 9cdd13b2e..82e0cc21e 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 a8526de83..c2b2376f8 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()); +} From ced8e167d0eacc5f8c5ca35c6f01c47cb5893050 Mon Sep 17 00:00:00 2001 From: Serial <69764315+Serial-ATA@users.noreply.github.com> Date: Tue, 19 Nov 2024 23:25:42 -0500 Subject: [PATCH 2/3] Tests: Allow missing docs --- lofty/tests/files/main.rs | 2 ++ lofty/tests/fuzz/main.rs | 2 ++ lofty/tests/hound.rs | 2 ++ lofty/tests/picture/main.rs | 2 ++ lofty/tests/tags/main.rs | 2 ++ 5 files changed, 10 insertions(+) diff --git a/lofty/tests/files/main.rs b/lofty/tests/files/main.rs index 2b81b12ba..8466dc9f7 100644 --- a/lofty/tests/files/main.rs +++ b/lofty/tests/files/main.rs @@ -1,3 +1,5 @@ +#![allow(missing_docs)] + mod aac; mod aiff; mod ape; diff --git a/lofty/tests/fuzz/main.rs b/lofty/tests/fuzz/main.rs index b92f38928..807f16e86 100644 --- a/lofty/tests/fuzz/main.rs +++ b/lofty/tests/fuzz/main.rs @@ -1,3 +1,5 @@ +#![allow(missing_docs)] + use lofty::config::ParseOptions; use lofty::prelude::*; diff --git a/lofty/tests/hound.rs b/lofty/tests/hound.rs index 3635107b8..65e6cd2f3 100644 --- a/lofty/tests/hound.rs +++ b/lofty/tests/hound.rs @@ -1,3 +1,5 @@ +#![allow(missing_docs)] + use lofty::config::ParseOptions; use lofty::error::Result; use lofty::iff::wav::WavFile; diff --git a/lofty/tests/picture/main.rs b/lofty/tests/picture/main.rs index 1584f9cf5..40098bd89 100644 --- a/lofty/tests/picture/main.rs +++ b/lofty/tests/picture/main.rs @@ -1,3 +1,5 @@ +#![allow(missing_docs)] + mod format_parsers; mod from_reader; mod information; diff --git a/lofty/tests/tags/main.rs b/lofty/tests/tags/main.rs index c4f855f3c..942f21c26 100644 --- a/lofty/tests/tags/main.rs +++ b/lofty/tests/tags/main.rs @@ -1 +1,3 @@ +#![allow(missing_docs)] + mod conversions; From 978708b6e50994f2e3e8e995302e8bc9396071eb Mon Sep 17 00:00:00 2001 From: Serial <69764315+Serial-ATA@users.noreply.github.com> Date: Tue, 19 Nov 2024 23:29:48 -0500 Subject: [PATCH 3/3] APE: Check subtraction for header APE tag length --- CHANGELOG.md | 1 + lofty/src/ape/read.rs | 6 +++++- lofty/tests/fuzz/apefile_read_from.rs | 11 +++++++++++ ...3119c37ca5982277fc75787a0a3c34aadbca7_minimized | Bin 0 -> 90 bytes lofty/tests/fuzz/main.rs | 1 + 5 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 lofty/tests/fuzz/apefile_read_from.rs create mode 100644 lofty/tests/fuzz/assets/apefile_read_from/crash-6373119c37ca5982277fc75787a0a3c34aadbca7_minimized diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e5a7eff7..8d57e6e6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **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)) + - **APE**: Fix panic when parsing incorrectly sized header APE tags ([issue](https://github.com/Serial-ATA/lofty-rs/issues/481)) ## [0.21.1] - 2024-08-28 diff --git a/lofty/src/ape/read.rs b/lofty/src/ape/read.rs index 675cace47..9a5b4da23 100644 --- a/lofty/src/ape/read.rs +++ b/lofty/src/ape/read.rs @@ -86,7 +86,11 @@ where } let ape_header = read_ape_header(data, false)?; - stream_len -= u64::from(ape_header.size); + let Some(new_stream_length) = stream_len.checked_sub(u64::from(ape_header.size)) + else { + err!(SizeMismatch); + }; + stream_len = new_stream_length; if parse_options.read_tags { let ape = read_ape_tag_with_header(data, ape_header, parse_options)?; diff --git a/lofty/tests/fuzz/apefile_read_from.rs b/lofty/tests/fuzz/apefile_read_from.rs new file mode 100644 index 000000000..0fa68b089 --- /dev/null +++ b/lofty/tests/fuzz/apefile_read_from.rs @@ -0,0 +1,11 @@ +use lofty::ape::ApeFile; +use lofty::config::ParseOptions; +use lofty::file::AudioFile; + +#[test_log::test] +fn panic1() { + let mut reader = crate::get_reader( + "apefile_read_from/crash-6373119c37ca5982277fc75787a0a3c34aadbca7_minimized", + ); + let _ = ApeFile::read_from(&mut reader, ParseOptions::default()); +} diff --git a/lofty/tests/fuzz/assets/apefile_read_from/crash-6373119c37ca5982277fc75787a0a3c34aadbca7_minimized b/lofty/tests/fuzz/assets/apefile_read_from/crash-6373119c37ca5982277fc75787a0a3c34aadbca7_minimized new file mode 100644 index 0000000000000000000000000000000000000000..65841505c95fab351318d1f893d4c358039c3d61 GIT binary patch literal 90 zcmeZtF=l4i%D}*26yh0T1f*FRS{QutixWef{r!Mk;(%j-Ylx$}YlND(qmL6X D^}7km literal 0 HcmV?d00001 diff --git a/lofty/tests/fuzz/main.rs b/lofty/tests/fuzz/main.rs index 807f16e86..1ce29900d 100644 --- a/lofty/tests/fuzz/main.rs +++ b/lofty/tests/fuzz/main.rs @@ -10,6 +10,7 @@ use std::time::Instant; mod aacfile_read_from; mod aifffile_read_from; +mod apefile_read_from; mod flacfile_read_from; mod id3v2; mod mp4file_read_from;