Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 39 additions & 14 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,18 @@ fn read_vds_checked<R: Read + Seek>(
return Ok(None);
};

// Both operands are u32 fields from the descriptors, so their sum can leave
// the address space. A wrapped LBA would point at a real but wrong block --
// a confidently incorrect File Set Descriptor -- so an overflow means the
// descriptor pair does not describe a location and there is nothing to
// report.
let Some(fsd_lba) = partition_start.checked_add(fsd) else {
return Ok(None);
};

Ok(Some(VdsInfo {
partition_start,
fsd_lba: partition_start + fsd,
fsd_lba,
partition_kind: kind,
map_count,
}))
Expand Down Expand Up @@ -593,25 +602,41 @@ fn parse_fids<R: Read + Seek>(

if file_chars & FC_PARENT == 0 {
let is_dir = file_chars & FC_DIRECTORY != 0;
let fe_lba = partition_start + icb_lbn;

let id_start = off + tag_size + 20 + impl_use_len;
let id_end = (id_start + file_id_len).min(data.len());
// The name-field bounds are header-derived too. Saturating is right
// here rather than skipping: an out-of-range span clamps to the end
// of the buffer, and the `id_end > id_start` test below then leaves
// the name empty instead of decoding anything.
let id_start = off
.saturating_add(tag_size)
.saturating_add(20)
.saturating_add(impl_use_len);
let id_end = id_start.saturating_add(file_id_len).min(data.len());
let name = if id_end > id_start {
decode_osta_cs0(&data[id_start..id_end])
} else {
String::new()
};

// Read the FE to get the canonical file size.
let size = read_fe_info_len(reader, block_size, fe_lba).unwrap_or(0);

entries.push(UdfFileEntry {
name,
is_dir,
size,
fe_lba,
});
// Same hazard as the FSD address above, per directory entry. Both
// operands are u32 from the image, so their sum can leave the
// address space; a wrapped LBA would send the File Entry read at a
// real but wrong block and report a confidently incorrect size.
//
// Dropping the entry rather than `continue`-ing is deliberate: the
// loop advances `off` below this block, so skipping the iteration
// would leave the cursor where it was and spin forever on the same
// descriptor.
if let Some(fe_lba) = partition_start.checked_add(icb_lbn) {
// Read the FE to get the canonical file size.
let size = read_fe_info_len(reader, block_size, fe_lba).unwrap_or(0);

entries.push(UdfFileEntry {
name,
is_dir,
size,
fe_lba,
});
}
}

off += fid_advance.max(4);
Expand Down
Binary file not shown.
Binary file added tests/data/fuzz-crash-read_dir-add-overflow.bin
Binary file not shown.
59 changes: 59 additions & 0 deletions tests/fuzz_regressions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
//! Inputs that once panicked the parser, kept as ordinary tests.
//!
//! These are real reproducers minted by the nightly fuzz job, not hand-authored
//! fixtures — they are the bytes libFuzzer actually found, downloaded from the
//! failing run's artifact. Both crashed with `attempt to add with overflow` on a
//! logical-block address computed from two header fields.
//!
//! They live here rather than only in `fuzz/corpus/` so that `cargo test` fails
//! if the bound regresses. A defect found by fuzzing should not need fuzzing to
//! be caught a second time: the fuzz job runs nightly, the test suite runs on
//! every push.
//!
//! The assertion is deliberately weak on *what* comes back. A malformed image
//! may legitimately parse to `None`, to an error, or even to a structure — the
//! contract under test is only that it returns at all rather than panicking.

// An integration test is a separate crate, so the workspace's panic-free lints
// do not reach it and it carries its own allow — the same shape the fleet
// standard prescribes. A fixture that fails to load should fail loudly.
#![allow(clippy::expect_used)]

use std::io::Cursor;

/// libFuzzer `parse_state` reproducer: overflowed `partition_start + fsd_lbn`
/// while resolving the File Set Descriptor location.
const PARSE_STATE_CRASH: &[u8] = include_bytes!("data/fuzz-crash-parse_state-add-overflow.bin");

/// libFuzzer `read_dir` reproducer: overflowed `partition_start + icb_lbn`
/// while walking a directory's File Identifier Descriptors.
const READ_DIR_CRASH: &[u8] = include_bytes!("data/fuzz-crash-read_dir-add-overflow.bin");

#[test]
fn parse_state_reproducer_does_not_panic() {
let _ = udf_forensic::parse_udf_state_checked(&mut Cursor::new(PARSE_STATE_CRASH));
let _ = udf_forensic::parse_udf_state(&mut Cursor::new(PARSE_STATE_CRASH));
}

/// The `read_dir` harness steers addressing from the leading bytes rather than
/// parsing them out of the image, so the reproducer has to be decoded the same
/// way its fuzz target does or it exercises nothing.
#[test]
fn read_dir_reproducer_does_not_panic() {
const BLOCK_SIZES: [u32; 4] = [2048, 512, 1024, 4096];
let (&sel, rest) = READ_DIR_CRASH
.split_first()
.expect("reproducer is non-empty");
assert!(rest.len() >= 8, "reproducer carries the addressing prefix");

let block_size = BLOCK_SIZES[(sel as usize) % BLOCK_SIZES.len()];
let partition_start = u32::from_le_bytes([rest[0], rest[1], rest[2], rest[3]]);
let dir_fe_lba = u32::from_le_bytes([rest[4], rest[5], rest[6], rest[7]]);

let _ = udf_forensic::read_dir_at_lba(
&mut Cursor::new(&rest[8..]),
block_size,
partition_start,
dir_fe_lba,
);
}
Loading