Repository navigation
Three panics from fuzzing: unchecked slice indexing in RAF, X3F, and TIFF parsers #54
Description
Activity
- added a commit that references this issue
on Apr 7, 2026 rawloader was extensively fuzzed to the point I was convinced it was free of issues. I even created some fake formats to make sure the fuzzing of the decoders themselves was exercised instead of fuzzing mostly exercising the format parsing. But it does rely on catch_unwind to not have to sprinkle the code with checks. Why do it painstakingly manually when the compiler does it perfectly instead? Unfortunately that means it is not suitable for usage under panic=abort for most cases. I would love to use the code already in the compiler to fix this by not having to unwind but instead do the conversion to error directly at the point where the compiler is already adding the panic code. Unfortunately there was no mechanism to do this last time I checked.
The only way I've considered changing this is if there was at least a robust way to disallow panics completely at the crate level and have the compiler fail the compile whenever it was about to insert a panic check and tell you why. That way even though the checks would still be introduced manually they would be enforced by the compiler and easier to implement cleanly. Playing whack-a-mole with fuzzing is an even worse situation than having the unwind check. I wrote this entire crate in less time than I had already spent doing that to the equivalent C++ code. Last time I looked into it there were some hackish crates to achieve some of that. Maybe that's improved?
Thanks for the thoughtful response! The
catch_unwindapproach makes sense as a pragmatic tradeoff — the compiler is indeed already doing bounds checking, and manually duplicating that logic is tedious and error-prone.A few options that have improved since you last looked, in case any are useful:
1. Clippy deny lints (lowest friction)
[lints.clippy] indexing_slicing = "deny"
This flags every
buf[pos..pos+4]pattern and suggests.get(). It's not a compile-time guarantee, but it catches the common case in CI and forces explicit handling at each site. Combined withunwrap_used = "deny"andexpect_used = "deny", it covers most panic sources.2.
#[no_panic](dtolnay) for critical helpersThe
no_paniccrate uses a linker trick to fail the build if any panic paths exist in a function. Applying it toBEu32,LEu16, etc. after converting them to returnOption<T>would give a compile-time guarantee that those specific functions can't panic — the linker enforces it. It works at function granularity, so you don't have to convert the whole crate at once.3. The helpers approach
Rather than whack-a-mole at each call site, converting the ~10 byte-reading helpers in
basics.rsto returnOption<T>(using.get()internally) would fix the systemic issue in one place. Callers that already handle errors use?, callers in fire-and-forget paths use.unwrap_or(0). This is a bounded refactor (~10 function signatures + their callers) rather than touching every decode site individually.Happy to prepare a PR for option 3 if you'd be interested — it would be a focused change to
basics.rs+ callers, keeping the existing architecture intact.- added a commit that references this issue
on Apr 8, 2026 I've put together a PR implementing option 3 (the helpers approach) at #56.
The helpers return 0 on out-of-bounds rather than changing signatures to
Option<T>— this keeps all callers unchanged.cargo asmconfirms LLVM eliminates every bounds check in release builds, producing identical assembly. The targeted fixes inTiffEntry::new()and the IFD loop catch the cases where a zero return would cause downstream issues.- added a commit that references this issue
on Apr 8, 2026
While fuzzing a downstream consumer of rawloader 0.37.1, I found three distinct panics caused by unchecked slice indexing on crafted inputs. These are caught by rawloader's internal
catch_unwindinRawLoader::decode(), but become aborts underpanic=abort(which is the default for release profiles in many projects and all WASM targets).1. RAF (FUJIFILM header):
basics.rs:64—BEu32out of boundsTiffIFD::new_file()intiff.rs:128matches theFUJIFILMmagic atbuf[0..8]and immediately callsBEu32(buf, 84)without checking that the buffer is long enough.Minimal input: 8 bytes — just the ASCII string
FUJIFILM.Suggested fix: Check
buf.len() >= 108(or similar minimum) before accessing offsets 84, 92, and 100 in the FUJIFILM branch ofnew_file().2. X3F (FOVb header):
x3f.rs:37— slice index out of boundsX3fFile::new()reads a 4-byte directory offset from the end of the file (LEu32(&buf.buf, buf.size-4)) and uses it as a slice index without bounds checking. A crafted file can set this to any value (e.g.,0xf1f1f1f1), causing an OOB panic at&buf.buf[offset..].Additionally,
X3fDecoder::new()at line 100 calls.unwrap()on theX3fFile::new()result.Minimal input: 257 bytes —
FOVbmagic + padding with0xf1bytes.Suggested fix:
if offset >= buf.size { return Err(...) }before the slice at line 37..unwrap()to?inX3fDecoder::new().3. TIFF with large IFD count:
basics.rs:80—LEu16out of boundsTiffIFD::new()reads the IFD entry count attiff.rs:196and caps it at 4000, but does not validate that the buffer actually containsnum * 12 + 2 + 4bytes from the offset. Whennumis large relative to the actual buffer size, the loop reads past the end.Minimal input: 1026 bytes — valid TIFF little-endian header with IFD offset 8, entry count 136, but only enough data for ~84 entries.
Suggested fix: Validate
offset + 2 + (num as usize) * 12 + 4 <= buf.len()before entering the entry loop.Notes
I understand that
RawLoader::decode()wraps the decode path incatch_unwind, which masks these panics in the default API. However:catch_unwinddoes not work underpanic=abort, which is common in release builds and required for WASM.get_decoder()andTiffIFD::new_file()methods are public and can be called without thecatch_unwindwrapper.All three inputs are available as minimized fuzz artifacts — happy to provide the exact bytes if helpful. Thank you for maintaining rawloader!