Infinite loop on malformed input

[Shnatsel]

Decoding these samples with Decoder.read_image() causes 100% CPU usage. I've run it for 10 minutes before giving up. The code is likely entering an infinite loop.

The exact reproduction code can be found in #28. Found via AFL.rs, tested on image-tiff version 0.2.2

[197g]

How did you detect the infinite loop with AFL here? Such a detection seems generally useful for some other fuzzing projects.

[Shnatsel]

AFL has categorized it as a hang. It monitors how much time has passed since execution for a given sample started and kills the process if it takes too long. The exact timeout is adjustable through a command-line parameter.

[birktj]

Backtrace from gdb when entering infinite loop:

I am not too used to this kind of debugging, but I am reasonably sure that it is hanging on one instruction, I step through in gdb, and then it suddenly stops. Could this error possibly come from outside of this crate?

#0  0x00007ffff7f59184 in read () from /usr/lib/libpthread.so.0
#1  0x00005555555e9993 in std::sys::unix::fd::FileDesc::read () at src/libstd/sys/unix/fd.rs:58
#2  std::sys::unix::fs::File::read () at src/libstd/sys/unix/fs.rs:564
#3  <std::fs::File as std::io::Read>::read () at src/libstd/fs.rs:608
#4  0x000055555557a01c in <tiff::decoder::stream::SmartReader<R> as std::io::Read>::read (self=0x7ffff7bdd9c0, buf=...) at /home/birk/dev/github/image-tiff/src/decoder/stream.rs:177
#5  0x0000555555578d72 in std::io::impls::<impl std::io::Read for &'a mut R>::read (self=0x7ffff7bdb788, buf=...)
    at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/io/impls.rs:23
#6  0x0000555555579197 in tiff::decoder::stream::read_packbits_run (reader=0x7ffff7bdb788, buffer=0x7ffff7bdb7a0) at /home/birk/dev/github/image-tiff/src/decoder/stream.rs:116
#7  0x0000555555578e24 in tiff::decoder::stream::PackBitsReader::new (reader=0x7ffff7bdd9c0, byte_order=tiff::decoder::stream::ByteOrder::BigEndian, length=1)
    at /home/birk/dev/github/image-tiff/src/decoder/stream.rs:104
#8  0x00005555555822f1 in <tiff::decoder::Decoder<R>>::expand_strip (self=0x7ffff7bdd938, buffer=..., offset=16151, length=1, max_uncompressed_length=23707)
    at /home/birk/dev/github/image-tiff/src/decoder/mod.rs:493
#9  0x0000555555584f98 in <tiff::decoder::Decoder<R>>::read_strip_to_buffer (self=0x7ffff7bdd938, buffer=...) at /home/birk/dev/github/image-tiff/src/decoder/mod.rs:579
#10 0x0000555555580c0a in <tiff::decoder::Decoder<R>>::read_image (self=0x7ffff7bdd938) at /home/birk/dev/github/image-tiff/src/decoder/mod.rs:646
#11 0x00005555555655fa in fuzz_tests::decode_tiff (file=...) at tests/fuzz_tests.rs:17
#12 0x0000555555592b2b in fuzz_tests::inf_loop::{{closure}} (file=...) at tests/fuzz_tests.rs:39
#13 0x000055555559203f in fuzz_tests::test_directory (path=..., f=...) at tests/fuzz_tests.rs:11
#14 0x0000555555565785 in fuzz_tests::inf_loop () at tests/fuzz_tests.rs:38
#15 0x0000555555592afa in fuzz_tests::inf_loop::{{closure}} () at tests/fuzz_tests.rs:37
#16 0x000055555558ad7e in core::ops::function::FnOnce::call_once () at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libcore/ops/function.rs:238
#17 0x000055555559d45f in test::run_test::{{closure}} () at src/libtest/lib.rs:1471
#18 core::ops::function::FnOnce::call_once () at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libcore/ops/function.rs:238
#19 <F as alloc::boxed::FnBox<A>>::call_box () at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/liballoc/boxed.rs:673
#20 0x000055555560365a in __rust_maybe_catch_panic () at src/libpanic_unwind/lib.rs:102
#21 0x0000555555595294 in std::panicking::try () at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/panicking.rs:289
#22 std::panic::catch_unwind () at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/panic.rs:398
#23 test::run_test::run_test_inner::{{closure}} () at src/libtest/lib.rs:1426
#24 std::sys_common::backtrace::__rust_begin_short_backtrace () at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/sys_common/backtrace.rs:136
#25 0x0000555555596145 in std::thread::Builder::spawn_unchecked::{{closure}}::{{closure}} () at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/thread/mod.rs:477
#26 <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once () at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/panic.rs:319
#27 std::panicking::try::do_call () at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/panicking.rs:310
#28 0x000055555560365a in __rust_maybe_catch_panic () at src/libpanic_unwind/lib.rs:102
#29 0x000055555559d26d in std::panicking::try () at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/panicking.rs:289
#30 std::panic::catch_unwind () at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/panic.rs:398
#31 std::thread::Builder::spawn_unchecked::{{closure}} () at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/thread/mod.rs:476
#32 <F as alloc::boxed::FnBox<A>>::call_box () at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/liballoc/boxed.rs:673
#33 0x00005555555f75de in _$LT$alloc..boxed..Box$LT$$LP$dyn$u20$alloc..boxed..FnBox$LT$A$C$$u20$Output$u3d$R$GT$$u20$$u2b$$u20$$u27$a$RP$$GT$$u20$as$u20$core..ops..function..FnOnce$LT$A$GT$$GT$::call_once::hece536cf07b94f8d () at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/liballoc/boxed.rs:683
#34 std::sys_common::thread::start_thread () at src/libstd/sys_common/thread.rs:24
#35 std::sys::unix::thread::Thread::new::thread_start () at src/libstd/sys/unix/thread.rs:90
#36 0x00007ffff7f4fa9d in start_thread () from /usr/lib/libpthread.so.0
#37 0x00007ffff7e65b23 in clone () from /usr/lib/libc.so.6

[Shnatsel]

Sadly I've never investigated anything like this using a debugger. I usually leave some disabled-by-default logging in the code to be able to see where execution goes and dump intermediate values.

I'll see if I can capture an execution trace with perf and maybe trace it to a specific function that way.

[Shnatsel]

Okay, I've recorded a trace with perf and opened it in https://github.com/KDAB/hotspot

You can see that it does actually get to read_image(), gets into this crate's SmartReader, and from there it's just calls to std::io::Cursor. This is not particularly helpful, but at least we know which functions actually get executed... which you could see in the gdb backtrace anyway.

[Shnatsel]

I've put a eprintln!("called") in read_packbits_run(), and it's getting invoked over and over, so the bug is definitely in this crate and in something outside read_packbits_run() in the backtrace.

[197g]

https://github.com/PistonDevelopers/image-tiff/blob/ff2621753477252e9a9abdb03edd6fef1808300b/src/decoder/stream.rs#L103-L105 doesn't break on Ok(0) which would indicate a finished stream. On file end, we just loop infinitely as the OS won't stop us from trying to read further.

Edit: I'm pretty sure this is Rusts worst API mistake, there should have been some encapsulation for this ^^

[197g]

It should probably return Err(io::ErrorKind::UnexpectedEof.into()) instead when it read no data?

[birktj]

I am working on a PR with a bunch of bug fixes, I will try too put a fix for this in it.