Panic in jpeg format loading cnn.com in Servo

[jdm]

thread 'ImageCacheThread' panicked at 'arithmetic operation overflowed', /Users/jdm/.cargo/registry/src/github.com-1ecc6299db9ec823/immeta-0.3.2/src/formats/jpeg.rs:37
stack backtrace:
   1:        0x10ac3df18 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
   2:        0x10ac443a5 - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
   3:        0x10ac43fbe - std::panicking::default_hook::hc2c969e7453d080c
   4:        0x10a119e19 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::h241fdc1bd2e9f090
   5:        0x10a119cb3 - _<std..thread..LocalKey<T>>::with::h7b8c266a788a1380
   6:        0x10a119b0a - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::h49531732a5b6b1e1
   7:        0x10ac2bb52 - std::panicking::rust_panic_with_hook::hfe203e3083c2b544
   8:        0x10ac44966 - std::panicking::begin_panic::h4889569716505182
   9:        0x10ac2d448 - std::panicking::begin_panic_fmt::h484cd47786497f03
  10:        0x10ac445bf - rust_begin_unwind
  11:        0x10ac6bdb0 - core::panicking::panic_fmt::h257ceb0aa351d801
  12:        0x10ac6c0ac - core::panicking::panic::h4bb1497076d04ab9
  13:        0x10869cb02 - _<formats..jpeg..Metadata as traits..LoadableMetadata>::load::h1e940b8d58d4471e
  14:        0x10867a000 - immeta::generic::load::hecc3105c1cd6a3b3
  15:        0x10867978b - immeta::generic::load_from_buf::h99f4a09595977935
  16:        0x10855a5e3 - net::image_cache_thread::ImageCache::handle_progress::hb85efed105d4ae3a
  17:        0x108540fe0 - net::image_cache_thread::ImageCache::run::h8a7f9c4cea47c0ce
  18:        0x1085942c4 - net::image_cache_thread::new_image_cache_thread::_$u7b$$u7b$closure$u7d$$u7d$::hd955388743fd3934
  19:        0x1085941c3 - _<std..panic..AssertUnwindSafe<F> as std..ops..FnOnce<()>>::call_once::he46335d08fc20bad
  20:        0x10859410b - std::panicking::try::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::h47315e3d894b7f3e
  21:        0x108594834 - std::panicking::try::call::h79a9ea3aa322847c
  22:        0x10ac47b2b - __rust_try
  23:        0x10ac47ac5 - __rust_maybe_catch_panic
  24:        0x10859405d - std::panicking::try::_$u7b$$u7b$closure$u7d$$u7d$::hdd5242960360b596
  25:        0x108593f9f - _<std..thread..LocalKey<T>>::with::h1ed8404e3e27cf1e
  26:        0x108593ded - std::panicking::try::h4b8e2e54c75fdc17
  27:        0x108593cca - std::panic::catch_unwind::h7a980a0a21f064ec
  28:        0x108593b26 - std::thread::Builder::spawn::_$u7b$$u7b$closure$u7d$$u7d$::h87e5067523346194
  29:        0x108594a2c - _<F as std..boxed..FnBox<A>>::call_box::ha6e60ff952ac3d25
  30:        0x10ac433c8 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  31:     0x7fff8f94a898 - _pthread_body
  32:     0x7fff8f94a729 - _pthread_start

This is

immeta/src/formats/jpeg.rs

Line 37 in f06b1fd

try_if_eof!(r.read_u16::<BigEndian>(), "when reading marker payload size") - 2

. Presumably we end up with 1 or 0.

[netvl]

Wow, thanks! I'll investigate it ASAP. Is there a direct link to the offending image?

[jdm]

Not yet; sorry.

[jdm]

The image appears to be data:image/webp;base64,UklGRkoAAABXRUJQVlA4WAoAAAAQAAAAAAAAAAAAQUxQSAwAAAABBxAR/Q9ERP8DAABWUDggGAAAADABAJ0BKgEAAQADADQlpAADcAD++/1QAA== interestingly enough.

[jdm]

The jpeg loader was only tried because the webp one didn't return an Ok value, apparently.

[jdm]

&['R', 'I', 'F', 'F', 'J', '\0', '\0', '\0', 'W', 'E', 'B', 'P', 'V', 'P', '8', 'X', '\n', '\0', '\0', '\0', '\x10', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', 'A', 'L', 'P', 'H', '\f', '\0', '\0', '\0', '\x01', '\a', '\x10', '\x11', '�', '\x0f', 'D', 'D', '�', '\x03', '\0', '\0', 'V', 'P', '8', ' ', '\x18', '\0', '\0', '\0', '0', '\x01', '\0', '\x9d', '\x01', '*', '\x01', '\0', '\x01', '\0', '\x03', '\0', '4', '%', '�', '\0', '\x03', 'p', '\0', '�', '�', '�', 'P', '\0']

[netvl]

@jdm Thanks! Indeed this could happen, webp support is not full yet. Apparently it has VP8X chunk id which is not supported now. Do you want a quick fix which disables webp support for now or are you willing to wait until I add support for other webp chunk formats? I'll start working on it right away in the latter case, will probably finish it right after the weekend.

[jdm]

Perhaps make webp support a feature that can be enabled by users of the library? It would still be useful to harden the jpeg decoder to avoid this, too.

[netvl]

a feature

This somehow didn't even occur to me. Indeed, that would be a nice solution, thanks!

It would still be useful to harden the jpeg decoder to avoid this, too.

Yes, I agree. Unfortunately, JPEG is such a loose format that it is pretty hard to do it... I'll see what I can do.

[netvl]

Apparently I was wrong about the looseness of the JPEG format. I finally have found its specification, and I was able to improve the JPEG parser considerably. It does not contain the overflow problem anymore, and it also provides more pieces of metadata. See version 0.3.4.

immeta will now return "unsupported image format" error for the image in question. @jdm please tell if this is an acceptable behavior.

[jdm]

Perfect!

[netvl]

Okay, then I'll close this. Thanks!