Apparent use of uninitialized memory when rendering JPGs on <canvas>

RESOLVED FIXED in Firefox 48

Status

()

Core
ImageLib
RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: Craig Young, Assigned: tnikkel)

Tracking

({csectype-disclosure, sec-moderate})

42 Branch
mozilla48
x86_64
Mac OS X
csectype-disclosure, sec-moderate
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

(Whiteboard: [adv-main48-])

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Created attachment 8684431 [details]
ff_jpg_disclosure.tgz

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.80 Safari/537.36

Steps to reproduce:

I used afl-fuzz to generate interesting crafted JPG files by fuzzing an open source image parsing tool and then repeatedly rendered each of 4000+ test cases on a canvas using a harness script included in afl-fuzz.  (Concept borrowed from afl-fuzz author lcamtuf and referenced here: https://lcamtuf.blogspot.com/2014/10/two-more-browser-memory-disclosure-bugs.html )

The attached tgz can be extracted to a local filesystem and ffjpg.html loaded in Firefox to demonstrate the problem using Michael Zalewski's PoC with my test cases.  I have observed this behavior on an Ubuntu 14.04 FF build as well as FireFox 42 on OS X 10.11.1.


Actual results:

A handful of the invalid/crafted JPG files intermittently render into the <canvas>.


Expected results:

Rendering the same source image file onto a <canvas> repeatedly should yield the same result.  This non-deterministic behavior would seem to indicate that some memory may be used without initialization.  If this is the case, an attacker may be able to disclose data from memory.  (Depending on what's going on, this could lead to an ASLR bypass or sensitive information being revealed unintentionally.)
(Reporter)

Updated

3 years ago
Component: Untriaged → Security
Keywords: csectype-disclosure
OS: Unspecified → Mac OS X
Hardware: Unspecified → x86_64

Comment 1

3 years ago
Seth or njn, do you know what might be causing this and/or if it's imglib/gfx/canvas that would be at issue here? (picked imglib for now, feel free to move about)
Group: firefox-core-security → core-security
Component: Security → ImageLib
Flags: needinfo?(seth)
Flags: needinfo?(n.nethercote)
Product: Firefox → Core
I don't know much about either JPEG decoding or canvas, but I can try running the test case under Valgrind on Monday.
Flags: needinfo?(n.nethercote)
(Reporter)

Comment 3

3 years ago
In case it helps, the test cases were generated by fuzzing iconvert (from https://launchpad.net/ubuntu/+source/openimageio/1.4.14~dfsg0-1ubuntu2).  From the larger set of crashing test cases (4600+) a number of them were hitting a bug (or a few bugs) in libjpeg-turbo.  I am admittedly not too familiar with the Mozilla codebase, but it is my understanding that this library may be used for Firefox as well on at least some platforms.
There was some similar issue reported, maybe last year, where some JPEG decoding was just nondeterministic, but I don't recall the details unfortunately.
(bug 1050342 is what I was thinking of)
(Reporter)

Comment 6

3 years ago
The PoC also produces non-deterministic JPEG rendering on Firefox for Android as tested on Android 6 (Nexus 5X).
I did a few runs with Valgrind. In one of them I got this:

> ==20950== Syscall param writev(vector[...]) points to uninitialised byte(s)
> ==20950==    at 0x5B8316D: ??? (syscall-template.S:81)
> ==20950==    by 0xBDC1DFD: ??? (in /usr/lib/x86_64-linux-gnu/libxcb.so.1.1.0)
> ==20950==    by 0xBDC2190: ??? (in /usr/lib/x86_64-linux-gnu/libxcb.so.1.1.0)
> ==20950==    by 0xBDC2210: xcb_writev (in /usr/lib/x86_64-linux-gnu/libxcb.so.1.1.0)
> ==20950==    by 0x83F711D: _XSend (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
> ==20950==    by 0x83EC924: ??? (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
> ==20950==    by 0x83ECB49: XPutImage (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
> ==20950==    by 0x1291492A: _draw_image_surface (cairo-xlib-surface.c:1363)
> ==20950==    by 0x12916502: _cairo_xlib_surface_clone_similar (cairo-xlib-surface.c:1514)
> ==20950==    by 0x12952E12: _cairo_surface_clone_similar (cairo-surface.c:1767)
> ==20950==    by 0x1293E5C0: _cairo_pattern_acquire_surface_for_surface (cairo-pattern.c:2324)
> ==20950==    by 0x1293E5C0: _cairo_pattern_acquire_surface (cairo-pattern.c:2426)
> ==20950==    by 0x1293E9BA: _cairo_pattern_acquire_surfaces (cairo-pattern.c:2499)
> ==20950==    by 0x12918F4C: _cairo_xlib_surface_acquire_pattern_surfaces (cairo-xlib-surface.c:2304)
> ==20950==    by 0x12918F4C: _cairo_xlib_surface_composite (cairo-xlib-surface.c:2470)
> ==20950==    by 0x1294BBA4: _cairo_surface_composite (cairo-surface.c:1889)
> ==20950==    by 0x1294FCD7: _composite_rectangle (cairo-surface-fallback.c:762)
> ==20950==    by 0x1294FCD7: _clip_and_composite_trapezoids (cairo-surface-fallback.c:812)
> ==20950==    by 0x12950AC3: _cairo_surface_fallback_fill (cairo-surface-fallback.c:1213)
> ==20950==    by 0x12950C11: _cairo_surface_fill (cairo-surface.c:2357)
> ==20950==    by 0x12930D07: _cairo_gstate_fill (cairo-gstate.c:1290)
> ==20950==    by 0x12955D15: _moz_cairo_fill_preserve (cairo.c:2473)
> ==20950==    by 0x10B161E6: mozilla::gfx::DrawTargetCairo::DrawPattern(mozilla::gfx::Pattern const&, mozilla::gfx::StrokeOptions const&, mozilla::gfx::DrawOptions const&, mozilla::gfx::DrawTargetCairo::DrawPatternType, bool) (DrawTargetCairo.cpp:959)
> 
> ==20950==  Address 0x4592b23c is 252 bytes inside a block of size 16,384 alloc'd
> ==20950==    at 0x4C2DD44: memalign (vg_replace_malloc.c:763)
> ==20950==    by 0x4C2DE0C: posix_memalign (vg_replace_malloc.c:916)
> ==20950==    by 0x404F6C: moz_posix_memalign (mozalloc.cpp:152)
> ==20950==    by 0x1010BE6F: mozilla::VolatileBuffer::Init(unsigned long, unsigned long) (VolatileBufferFallback.cpp:35)
> ==20950==    by 0x10CE6DF5: mozilla::image::AllocateBufferForImage(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat) (imgFrame.cpp:77)
> ==20950==    by 0x10CED486: mozilla::image::imgFrame::InitForDecoder(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat, unsigned char, bool) (imgFrame.cpp:214)
> ==20950==    by 0x10CDD9FA: mozilla::image::Decoder::AllocateFrameInternal(unsigned int, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat, unsigned char, mozilla::image::imgFrame*) (Decoder.cpp:324)
> ==20950==    by 0x10CDE149: mozilla::image::Decoder::AllocateFrame(unsigned int, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat, unsigned char) (Decoder.cpp:268)
> ==20950==    by 0x10CF9DBD: mozilla::image::nsJPEGDecoder::WriteInternal(char const*, unsigned int) (nsJPEGDecoder.cpp:390)
> ==20950==    by 0x10CDD88D: mozilla::image::Decoder::Write(char const*, unsigned int) (Decoder.cpp:183)
> ==20950==    by 0x10CDE302: mozilla::image::Decoder::Decode(mozilla::image::IResumable*) (Decoder.cpp:128)
> ==20950==    by 0x10CE4512: mozilla::image::DecodePool::Decode(mozilla::image::Decoder*) (DecodePool.cpp:455)
> ==20950==    by 0x10CE59BE: mozilla::image::DecodePoolWorker::Run() (DecodePool.cpp:281)
> ==20950==    by 0x101ACB10: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:964)
> ==20950==    by 0x101D18FE: NS_ProcessNextEvent(nsIThread*, bool) (nsThreadUtils.cpp:297)
> ==20950==    by 0x104797DD: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) (MessagePump.cpp:355)
> ==20950==    by 0x1042EA10: MessageLoop::RunInternal() (message_loop.cc:234)
> ==20950==    by 0x1042ECA4: RunHandler (message_loop.cc:227)
> ==20950==    by 0x1042ECA4: MessageLoop::Run() (message_loop.cc:201)
> ==20950==    by 0x101A92DB: nsThread::ThreadFunc(void*) (nsThread.cpp:376)
> ==20950==    by 0x62774D8: _pt_root (ptthread.c:212)
> 
> ==20950==  Uninitialised value was created by a heap allocation
> ==20950==    at 0x4C2BC0F: malloc (vg_replace_malloc.c:299)
> ==20950==    by 0x10CDF0F4: operator new [] (mozalloc.h:260)
> ==20950==    by 0x10CDF0F4: mozilla::image::Downscaler::BeginFrame(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::Maybe<mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> > const&, unsigned char*, bool, bool) (Downscaler.cpp:109)
> ==20950==    by 0x10CF9E38: mozilla::image::nsJPEGDecoder::WriteInternal(char const*, unsigned int) (nsJPEGDecoder.cpp:403)
> ==20950==    by 0x10CDD88D: mozilla::image::Decoder::Write(char const*, unsigned int) (Decoder.cpp:183)
> ==20950==    by 0x10CDE302: mozilla::image::Decoder::Decode(mozilla::image::IResumable*) (Decoder.cpp:128)
> ==20950==    by 0x10CE4512: mozilla::image::DecodePool::Decode(mozilla::image::Decoder*) (DecodePool.cpp:455)
> ==20950==    by 0x10CE59BE: mozilla::image::DecodePoolWorker::Run() (DecodePool.cpp:281)
> ==20950==    by 0x101ACB10: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:964)
> ==20950==    by 0x101D18FE: NS_ProcessNextEvent(nsIThread*, bool) (nsThreadUtils.cpp:297)
> ==20950==    by 0x104797DD: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) (MessagePump.cpp:355)
> ==20950==    by 0x1042EA10: MessageLoop::RunInternal() (message_loop.cc:234)
> ==20950==    by 0x1042ECA4: RunHandler (message_loop.cc:227)
> ==20950==    by 0x1042ECA4: MessageLoop::Run() (message_loop.cc:201)
> ==20950==    by 0x101A92DB: nsThread::ThreadFunc(void*) (nsThread.cpp:376)
> ==20950==    by 0x62774D8: _pt_root (ptthread.c:212)
> ==20950==    by 0x4E3E6A9: start_thread (pthread_create.c:333)
> ==20950==    by 0x5B8CEEC: clone (clone.S:109)
>
Group: core-security → gfx-core-security
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: sec-moderate
(Assignee)

Comment 8

2 years ago
About half the jpegs in the testcase are very large (eg 65285x1535). The testcase creates 50 copies of the image, which uses a lot of memory, so we refuse to allocate a surface for them sometimes. When this happens nothing is drawn to the canvas. This is reported as a variant. I guess this can be used to determine how much memory is devoted to images. Not really sure what we should do about this from a Gecko viewpoint. Also not sure what the fuzzer should do to avoid reporting this as a potential use of uninitialized memory.

Of the remaining files that aren't huge there is one issue that is not fixed by bug 1257101, bug 1050342, and bug 1187420 (didn't test if the latter two fixed anything here, but they landed since the testcase was posted and deal with similar issues). It is not a use of uninitialized memory though. I'll upload a patch next.
(Assignee)

Comment 9

2 years ago
Created attachment 8731930 [details] [diff] [review]
patch

Even though this doesn't disclose memory contents we should still fix it so that fuzzers don't get false positives and hence can detect actual problems like this.
Assignee: nobody → tnikkel
Attachment #8731930 - Flags: review?(seth)
Comment on attachment 8731930 [details] [diff] [review]
patch

Review of attachment 8731930 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.
Attachment #8731930 - Flags: review?(seth) → review+
(Assignee)

Updated

2 years ago
Flags: needinfo?(seth)
(Reporter)

Comment 12

2 years ago
I have some thoughts on how to tweak the canvas harness.  I will do this and test with the entire synthesized corpus from AFL.  Thanks.
(Assignee)

Comment 13

2 years ago
You could load one image completely, then draw it, then load the next image, etc. Instead of loading all 50 images simultaneously. But loading 50 images simultaneously can put pressure on imagelib in a different way, so continuing to test in that way is helpful too. You could test if nothing is drawn to the canvas, and use that particular file for serialized testing. Or check if dimensions of the image and assume it's OOM if it's big enough.
(Reporter)

Comment 14

2 years ago
The dimension check is certainly a good idea.  I was also thinking that in the case of a true info leak, the graphic will probably render in a variety of ways as I have seen on another browser where an issue was confirmed.  I'm looking into how I might be able to save output from canvas.toDataURL() across runs to confirm the behavior.  I think localStorage should be good for this but it will add some complexity to the testing process.

Have all of the files been confirmed as non-security relevant such that I can discuss this freely or should I hold off until the bug is closed out?
(Assignee)

Comment 15

2 years ago
(In reply to Craig Young from comment #14)
> Have all of the files been confirmed as non-security relevant such that I
> can discuss this freely or should I hold off until the bug is closed out?

Yeah, everything in this bug can be discussed publicly.

If you need any help getting fuzzing to work better let me know.
https://hg.mozilla.org/mozilla-central/rev/5005b1604f26
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Group: gfx-core-security → core-security-release
Alias: CVE-2016-5269
Whiteboard: [adv-main48+]
Alias: CVE-2016-5269
Whiteboard: [adv-main48+] → [adv-main48-]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.