Skip to content
Snippets Groups Projects
Commit 2f85ec68 authored by Timothy Nikkel's avatar Timothy Nikkel
Browse files

Bug 1823614. Limit the number of scans we allow in jpeg images to some finite...

Bug 1823614. Limit the number of scans we allow in jpeg images to some finite value. r=gfx-reviewers,bradwerth a=reland

We want to provide some finite limit to prevent small jpeg files from being able to tie up cpus for a much larger time than their small size would normally allow. We want to choose a number high enough so that no sane jpeg file would approach it, unless it had been crafted to take advantage of this problem.

Skia's jpeg decoder limit's it to 100:

https://searchfox.org/mozilla-central/rev/f078cd02746b29652c134b144f0629d47e378166/gfx/skia/skia/src/codec/SkJpegDecoderMgr.cpp#33

The OSS_Fuzz targets for libjpeg-turbo limit it to 500:

https://bugzilla.mozilla.org/show_bug.cgi?id=1252196#c11

Differential Revision: https://phabricator.services.mozilla.com/D173120
parent 0b75230e
No related branches found
No related tags found
No related merge requests found
......@@ -67,6 +67,7 @@ METHODDEF(boolean) fill_input_buffer(j_decompress_ptr jd);
METHODDEF(void) skip_input_data(j_decompress_ptr jd, long num_bytes);
METHODDEF(void) term_source(j_decompress_ptr jd);
METHODDEF(void) my_error_exit(j_common_ptr cinfo);
METHODDEF(void) progress_monitor(j_common_ptr info);
// Normal JFIF markers can't have more bytes than this.
#define MAX_JPEG_MARKER_LENGTH (((uint32_t)1 << 16) - 1)
......@@ -101,6 +102,7 @@ nsJPEGDecoder::nsJPEGDecoder(RasterImage* aImage,
mBytesToSkip = 0;
memset(&mInfo, 0, sizeof(jpeg_decompress_struct));
memset(&mSourceMgr, 0, sizeof(mSourceMgr));
memset(&mProgressMgr, 0, sizeof(mProgressMgr));
mInfo.client_data = (void*)this;
mSegment = nullptr;
......@@ -160,6 +162,9 @@ nsresult nsJPEGDecoder::InitInternal() {
mInfo.mem->max_memory_to_use = static_cast<long>(
std::min<size_t>(SurfaceCache::MaximumCapacity(), LONG_MAX));
mProgressMgr.progress_monitor = &progress_monitor;
mInfo.progress = &mProgressMgr;
// Record app markers for ICC data
for (uint32_t m = 0; m < 16; m++) {
jpeg_save_markers(&mInfo, JPEG_APP0 + m, 0xFFFF);
......@@ -734,6 +739,17 @@ my_error_exit(j_common_ptr cinfo) {
longjmp(err->setjmp_buffer, static_cast<int>(error_code));
}
static void progress_monitor(j_common_ptr info) {
int scan = ((j_decompress_ptr)info)->input_scan_number;
// Progressive images with a very large number of scans can cause the decoder
// to hang. Here we use the progress monitor to abort on a very large number
// of scans. 1000 is arbitrary, but much larger than the number of scans we
// might expect in a normal image.
if (scan >= 1000) {
my_error_exit(info);
}
}
/*******************************************************************************
* This is the callback routine from the IJG JPEG library used to supply new
* data to the decompressor when its input buffer is exhausted. It juggles
......
......@@ -82,6 +82,7 @@ class nsJPEGDecoder : public Decoder {
public:
struct jpeg_decompress_struct mInfo;
struct jpeg_source_mgr mSourceMgr;
struct jpeg_progress_mgr mProgressMgr;
decoder_error_mgr mErr;
jstate mState;
......
<img src="nosuchurl">
<img src="jpg-progressive-1000.jpg">
image/test/reftest/jpeg/jpg-progressive-1000.jpg

33.6 KiB

......@@ -68,3 +68,6 @@ HTTP == webcam-simulacrum.html blue.html
== non-interleaved_progressive-2.jpg non-interleaved_progressive-2-white-ref.png
== red-bad-marker.jpg red.jpg
# check that we reject jpegs with > 1000 scans
== jpg-progressive-1000.html jpg-progressive-1000-ref.html
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment