Panic on invalid input to jpeg_decoder::Decoder::decode

Describe the bug

Panic could be triggered when passing jpeg_decoder::Decoder::decode with invalid input. Panic info is shown below: thread 'main' panicked at /home/test/.cargo/registry/src/mirrors.ustc.edu.cn-61ef6e0cd06fb9b8/jpeg-decoder-0.3.1/src/decoder/lossless.rs:112:40: attempt to subtract with overflow

Full stack backtrace: 0: rust_begin_unwind at /rustc/07688726805d5db0a4bca445a6651d09708041ea/library/std/src/panicking.rs:617:5 1: core::panicking::panic_fmt at /rustc/07688726805d5db0a4bca445a6651d09708041ea/library/core/src/panicking.rs:67:14 2: core::panicking::panic at /rustc/07688726805d5db0a4bca445a6651d09708041ea/library/core/src/panicking.rs:117:5 3: jpeg_decoder::decoder::lossless::<impl jpeg_decoder::decoder::Decoder>::decode_scan_lossless at /home/test/.cargo/registry/src/mirrors.ustc.edu.cn-61ef6e0cd06fb9b8/jpeg-decoder-0.3.1/src/decoder/lossless.rs:112:40 4: jpeg_decoder::decoder::Decoder::decode_internal at /home/test/.cargo/registry/src/mirrors.ustc.edu.cn-61ef6e0cd06fb9b8/jpeg-decoder-0.3.1/src/decoder.rs:415:46 5: jpeg_decoder::decoder::Decoder::decode::{{closure}} at /home/test/.cargo/registry/src/mirrors.ustc.edu.cn-61ef6e0cd06fb9b8/jpeg-decoder-0.3.1/src/decoder.rs:294:36 6: jpeg_decoder::worker::WorkerScope::with at /home/test/.cargo/registry/src/mirrors.ustc.edu.cn-61ef6e0cd06fb9b8/jpeg-decoder-0.3.1/src/worker/mod.rs:61:9 7: jpeg_decoder::decoder::Decoder::decode at /home/test/.cargo/registry/src/mirrors.ustc.edu.cn-61ef6e0cd06fb9b8/jpeg-decoder-0.3.1/src/decoder.rs:294:9 8: jpeg_decoder_poc::main at ./src/main.rs:5:11 9: core::ops::function::FnOnce::call_once at /rustc/07688726805d5db0a4bca445a6651d09708041ea/library/core/src/ops/function.rs:250:5

Expected behavior

Not panic. It could be an error reported to the users.

Test environment:

Version: jpeg-decoder = "0.3.1" OS: Ubuntu 20.04, 64 bit Target triple: x86_64-unknown-linux-gnu Rustc version: rustc 1.73.0-nightly (076887268 2023-08-17)

To Reproduce

The PoC to reproduce the bug:

fn main() {
    let p = "jpeg-decode-crash.xx";
    if let Ok(data) = std::fs::read(p) {
        let mut decoder = jpeg_decoder::Decoder::new(data.as_slice());
        let _ = decoder.decode();
    }
}

PoC input is attached: jpeg-decode-crash.xx.zip

From the spec:

H.1.2.1 Prediction At the beginning of the first line and at the beginning of each restart interval the prediction value of 2^(P – 1) is used, where P is the input precision. If the point transformation parameter (see A.4) is non-zero, the prediction value at the beginning of the first lines and the beginning of each restart interval is 2^(P – Pt – 1), where Pt is the value of the point transformation parameter. Each prediction is calculated with full integer arithmetic precision, and without clamping of either underflow or overflow beyond the input precision bounds. For example, if Ra and Rb are both 16-bit integers, the sum is a 17-bit integer. After dividing the sum by 2 (predictor 7, [note: example refers to predictor Px = (Ra + Rb)/2]), the prediction is a 16-bit integer. [...] The difference between the prediction value and the input is calculated modulo 2^16. In the decoder the difference is decoded and added, modulo 2^16, to the prediction.

and, related:

A.4 Point transform In the lossless mode of operation the point transform is applied to the input samples. In the difference coding of the hierarchical mode of operation the point transform is applied to the difference between the input component samples and the reference component samples. In both cases the point transform is an integer divide by 2^Pt, where Pt is the value of the point transform parameter (see B.2.3). [...] The output of the decoder is rescaled by multiplying by 2^Pt.

The current code is (in lossless.rs):

let diff = differences[i][0]; /* as i32 */
let prediction = 1 << (frame.precision /* as u8 */ - scan.point_transform /* as u8 */ - 1) as i32;
let result = ((prediction + diff) & 0xFFFF) as u16; // modulo 2^16
let result = result << scan.point_transform;

In the example file, frame.precision is 8, scan.point_transform is 12. Simple underflow problem. I might suggest, given the wording of A.4: let prediction = (1 << (frame.precision - 1) as i32) >> scan.point_transform as i32;. This properly truncates to 0, instead of 1.

Edit: Removing the latter stuff, because I don't think that's at all what the spec was talking about. That said, I've still got a few questions about how the implementation deals with point_transform over the course of the function, but I'm going to look deeper.

Thanks for looking into this! If you figure out a fix, please do open a PR!

image-rs / jpeg-decoder