1450353 - (CVE-2020-12422) Write and read beyond bounds caused by nsJPEGEncoder::emptyOutputBuffer()

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, enhancement, P1)

Description

[mozillabugs]

Attachment: bug_1091_poc_3.htm

nsJPEGEncoder::emptyOutputBuffer (image\encoders\jpeg\nsJPEGEncoder.cpp) can experience an integer overflow. When it does, it causes compressed data (which can be derived from a script-provided image) to be written far beyond the end of its output buffer.

The bug is on line 433, which doesn't check for overflow.

This bug is similar to https://bugzilla.mozilla.org/show_bug.cgi?id=1434490 .

Attached is a POC that demonstrates the overflow and write beyond bounds.

If FF survives long enough to execute the lambda function in the |e.toBlob()| call in the POC, the script will then be able to read data from beyond bounds by reading the Blob, since that data comes from memory pointed to by |nsJPEGEncoder::mImageBuffer|. Often, however, FF dies reading beyond bounds while attempting to copy the buffer into the Blob, usually in NS_CopySegmentToBuffer().


420: boolean // static
421: nsJPEGEncoder::emptyOutputBuffer(jpeg_compress_struct* cinfo)
422: {
423:   nsJPEGEncoder* that = static_cast<nsJPEGEncoder*>(cinfo->client_data);
424:   NS_ASSERTION(that->mImageBuffer, "No buffer to empty!");
425: 
426:   // When we're reallocing the buffer we need to take the lock to ensure
427:   // that nobody is trying to read from the buffer we are destroying
428:   ReentrantMonitorAutoEnter autoEnter(that->mReentrantMonitor);
429: 
430:   that->mImageBufferUsed = that->mImageBufferSize;
431: 
432:   // expand buffer, just double size each time
433:   that->mImageBufferSize *= 2;
434: 
435:   uint8_t* newBuf = (uint8_t*)realloc(that->mImageBuffer,
436:                                       that->mImageBufferSize);
437:   if (!newBuf) {
438:     // can't resize, just zero (this will keep us from writing more)
439:     free(that->mImageBuffer);
440:     that->mImageBuffer = nullptr;
441:     that->mImageBufferSize = 0;
442:     that->mImageBufferUsed = 0;
443: 
444:     // This seems to be the only way to do errors through the JPEG library.  We
445:     // pass an nsresult masquerading as an int, which works because the
446:     // setjmp() caller casts it back.
447:     longjmp(((encoder_error_mgr*)(cinfo->err))->setjmp_buffer,
448:             static_cast<int>(NS_ERROR_OUT_OF_MEMORY));
449:   }
450:   that->mImageBuffer = newBuf;
451: 
452:   cinfo->dest->next_output_byte = &that->mImageBuffer[that->mImageBufferUsed];
453:   cinfo->dest->free_in_buffer = that->mImageBufferSize - that->mImageBufferUsed;
454:   return 1;
455: }

Use the POC by starting FF, disabling D2D (if on Windows) (by setting gfx.direct2d.disable to |true|), and enabling large-surface support (c.f. https://bugzilla.mozilla.org/show_bug.cgi?id=1282074 ) by setting gfx.max-alloc-size == 2147483647 (0x7fffffff). Restart FF.

Now attach a debugger to FF and set a BP on line 433, above. Load the POC and wait for the BP at which the value of |that->mImageBufferSize| is 0x80000000. Now step line 433 and observe the overflow and subsequent allocation of a 0-length buffer for |newBuf| (and thus |mImageBuffer|, line 450) and the setting of the next writable buffer location (|next_output_byte|, line 452) to 0x80000000 beyond the end of the buffer.

Now set a BP on encode_one_block_simd() (in media\libjpeg\jchuff.c) and proceed. When you hit the BP, observe the function writing compressed image data into hyperspace at |state->next_output_byte|.


The POC generates random data that is generally uncompressible, and thus causes the JPEG compressor to generate a compressed data stream that is sufficiently-long to cause the overflow.

Comment 1

[Daniel Veditz [:dveditz]]

Tyson: please try this in an ASAN build. I'm not seeing a problem in a Mac nightly.

Comment 2

[Tyson Smith [:tsmith]]

Attachment: ASan_log.txt

Log created with m-c:
BuildID=20180331213254
SourceStamp=3f9a70b125f6fb9be2b209159657fd7ae5515c01

Note: this only work when gfx.max-alloc-size is set properly and it takes a few minutes to trigger (and uses a lot of memory)

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

This is unlikely to manifest due to the pref settings and our limitations on the maximum surface size. But I have put together a patch to check allocation size.

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1450353.

Comment 5

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9155948 [details]
Bug 1450353.

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Somewhat hard. To overflow the encoder buffer, you would need a very large source surface, which we restrict the maximum size of, or combine it with a flaw in the JPEG encoder, to produce >2GB output.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
• Which older supported branches are affected by this flaw?: All
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: Not hard, I think the patch will likely apply cleanly as this code doesn't change much.
• How likely is this patch to cause regressions; how much testing does it need?: Not likely, we are just adding a bounds check.

Comment 6

[mozillabugs]

Hi. I cannot get my Phabricator account to work to review this patch. Can you find someone to delete the account so I can recreate it? Thanks.

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 9155948 [details]
Bug 1450353.

sec-approval+

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/autoland/rev/955ac3fe48e84ef3814bd5053bb758608944cbb6

Please nominate this for Beta approval when you get a chance. It grafts cleanly as-landed. It does graft cleanly to ESR68 too, but I'm not sure this is really needed there given how close to EOL it is?

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

Changing the priority to p1 as the bug is tracked by a release manager for the current beta.
See What Do You Triage for more information

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/955ac3fe48e84ef3814bd5053bb758608944cbb6
https://hg.mozilla.org/mozilla-central/rev/955ac3fe48e8

Comment 11

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9155948 [details]
Bug 1450353.

Beta/Release Uplift Approval Request

• User impact if declined: Theoretical vulnerability to buffer overflow in the JPEG encoder
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We are just adding a bounds check, low risk.
• String changes made/needed:

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 9155948 [details]
Bug 1450353.

approved for 78 rc1

Comment 13

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-release/rev/f1c8364c72de

Comment 14

[Tom Ritter [:tjr]]

Attachment: advisory.txt