Crash [@ MOZ_PNG_do_expand_plte] with broken png file

RESOLVED FIXED in Firefox 27, Firefox OS v1.3

Status

()

Core
ImageLib
--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: decoder, Assigned: Glenn Randers-Pehrson)

Tracking

({crash, testcase})

Trunk
mozilla30
crash, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox26 unaffected, firefox27 verified, firefox28 verified, firefox29 fixed, firefox30 fixed, firefox-esr24 unaffected, b2g18 unaffected, b2g-v1.1hd unaffected, b2g-v1.2 unaffected, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

Details

(Whiteboard: [qa-])

Attachments

(4 attachments)

(Reporter)

Description

4 years ago
Created attachment 8341932 [details]
f427b6bee1acd1fea3ec953bc556a18a.png

The attached PNG file crashes for me in Firefox (built from mozilla-central 4bf430d990e5).


ASan trace:

==25902==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000002fc (pc 0x7fe90ea1eb35 sp 0x7fff61352f90 bp 0x0000000002fc T0)
    #0 0x7fe90ea1eb34 in MOZ_PNG_do_expand_plte media/libpng/pngrtran.c:4675
    #1 0x7fe90ea18af3 in MOZ_PNG_do_read_trans media/libpng/pngrtran.c:2162
    #2 0x7fe90ea174a6 in MOZ_PNG_push_proc_row media/libpng/pngpread.c:1155
    #3 0x7fe90ea16cda in MOZ_PNG_proc_IDAT_data media/libpng/pngpread.c:1108
    #4 0x7fe90ea0f800 in MOZ_PNG_push_read_IDAT media/libpng/pngpread.c:986
    #5 0x7fe90ea0bcdc in MOZ_PNG_proc_some_data media/libpng/pngpread.c:127
    #6 0x7fe90ea0bcdc in MOZ_PNG_process_data media/libpng/pngpread.c:40
    #7 0x7fe90a44d4a4 in mozilla::image::nsPNGDecoder::WriteInternal(char const*, unsigned int) image/decoders/nsPNGDecoder.cpp:356
    #8 0x7fe90a40b13d in mozilla::image::Decoder::Write(char const*, unsigned int) image/src/Decoder.cpp:107
    #9 0x7fe90a3f677a in mozilla::image::RasterImage::WriteToDecoder(char const*, unsigned int) image/src/RasterImage.cpp:2150
    #10 0x7fe90a3ff9b4 in mozilla::image::RasterImage::DecodePool::DecodeSomeOfImage(mozilla::image::RasterImage*, mozilla::image::RasterImage::DecodePool::DecodeType, unsigned int) image/src/RasterImage.cpp:3415
    #11 0x7fe90a3fb51a in mozilla::image::RasterImage::DecodePool::DecodeABitOf(mozilla::image::RasterImage*) image/src/RasterImage.cpp:3215
    #12 0x7fe90a3fae47 in mozilla::image::RasterImage::RequestDecodeCore(mozilla::image::RasterImage::RequestDecodeType) image/src/RasterImage.cpp:2319
    #13 0x7fe90c14cb30 in nsImageLoadingContent::OnStopRequest(imgIRequest*, tag_nsresult) content/base/src/nsImageLoadingContent.cpp:251
[...]
Severity: major → critical
crashes on same address on windows: bp-2084589a-45fa-48cf-87a7-27de52131211
From the crashing line I'm assuming palette is null, and that the offset read can be controlled by the size of the image. Can it be arbitrarily large in an attempt to read from some valid section of memory? But if the loop is going to wind down toward zero eventually then maybe this is a guaranteed crash and not a useful information disclosure.
(Assignee)

Comment 3

4 years ago
A zero-length or NULL PLTE chunk should reach pngset.c line 535 which triggers a "benign error", that is treated in libpng-1.6.7 by default as a warning. I'm seeing the crash on Ubuntu (64-bit) with the current nightly (2013-12-10, 29.0a1, using the bundled libpng-1.6.7 in media/libpng).  It seems that this is a libpng design error that will need to be fixed upstream.  Meanwhile, I'm testing a mozilla patch that simply

changes line 530 of pngset.c from 
png_chunk_report(png_ptr, "Invalid palette", PNG_CHUNK_ERROR);
to
png_chunk_error(png_ptr, "Invalid palette");

Firefox 25.0.1-Ubuntu does not appear to have the bug.
(Assignee)

Updated

4 years ago
Assignee: nobody → glennrp+bmo
OS: Linux → All
Hardware: x86_64 → All
(Assignee)

Updated

4 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 4

4 years ago
Created attachment 8346300 [details] [diff] [review]
v00-945912 fix handling of incorrect palette length in pngset.c

Current mozilla-central with the v00 patch does not crash on my Ubuntu platform.  It ignores the empty PLTE chunk and displays the image.  There's no libpng warning because we suppress them.
(Assignee)

Comment 5

4 years ago
The bug was introduced to mozilla-central by checkin of libpng-1.6.6, bug #841734, on 26 September 2013.
I think this means that Firefox 27.0 is vulnerable and Firefox 26.0 and earlier are not. Libpng is planning to release the bugfix, in version 1.6.8, on Thursday 19 December 2013.  I have reported the libpng vulnerability to CERT (Tracking ID VRF#HP4TV3J2).

BTW, are the dates in https://wiki.mozilla.org/Releases all off by one year?
See Also: → bug 841734
(Assignee)

Updated

4 years ago
See Also: bug 841734
status-b2g18: --- → unaffected
status-b2g-v1.1hd: --- → unaffected
status-firefox26: --- → unaffected
status-firefox27: --- → affected
status-firefox28: --- → affected
status-firefox29: --- → affected
status-firefox-esr24: --- → unaffected
(Assignee)

Comment 6

4 years ago
(In reply to Daniel Veditz [:dveditz] from comment #2)
> From the crashing line I'm assuming palette is null, and that the offset
> read can be controlled by the size of the image. Can it be arbitrarily large
> in an attempt to read from some valid section of memory?

png_ptr->palette is always initialized to zero by png_create_read_struct().
"sp" is a png_byte, so "*sp" is in the range 0-255.  The size of the image
controls where "sp" is read from (a valid location in the row data), but reading
from "*sp" (NULL dereference of png_ptr->palette which is a png_colorp struct that
has three one-byte entries, red, green, and blue) is always in the range 0-767.
(Reporter)

Comment 7

4 years ago
(In reply to Glenn Randers-Pehrson from comment #6)
> is always in the range 0-767.

That range should still be safe for reading and not provide an opportunity to read valid memory for malicious purposes. Opening up the bug, thanks for the analysis! :)
Group: core-security
(Assignee)

Comment 8

4 years ago
This bug is CVE-2013-6954 and VU#650142.  It can be fixed either by applying the v00 patch or by upgrading libpng to version 1.6.8, or both.  I'll open a separate bug to upgrade libpng.
(Assignee)

Updated

4 years ago
Depends on: 952505
(Assignee)

Comment 9

4 years ago
Needs try server run.  If applied, this will bit rot the libpng-1.6.8 patch slightly.
Flags: needinfo?(ryanvm)
(Assignee)

Comment 11

4 years ago
try server results look pretty good, all green except for one timeout that I can't tell was related to this patch.
Not at all.
(Assignee)

Comment 13

4 years ago
Comment on attachment 8346300 [details] [diff] [review]
v00-945912 fix handling of incorrect palette length in pngset.c

try server results look pretty good, all green except for one timeout that I can't tell was related to this one-line patch.
Attachment #8346300 - Flags: review?(jmuizelaar)
Attachment #8346300 - Flags: review?(jmuizelaar) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e96704e1a7ea

Please nominate this for uplift to the affected branches.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
(Assignee)

Comment 16

4 years ago
Comment on attachment 8346300 [details] [diff] [review]
v00-945912 fix handling of incorrect palette length in pngset.c

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Upgrade to libpng-1.6.6 bug #886499
User impact if declined: DoS via a malformed PNG that has a zero-length PLTE chunk
Testing completed (on m-c, etc.): Tested on m-c with attached test case
Risk to taking this patch (and alternatives if risky): Low; it's a one-line change
String or IDL/UUID changes made by this patch:
Attachment #8346300 - Flags: approval-mozilla-release?
Attachment #8346300 - Flags: approval-mozilla-beta?
Attachment #8346300 - Flags: approval-mozilla-aurora?
Comment on attachment 8346300 [details] [diff] [review]
v00-945912 fix handling of incorrect palette length in pngset.c

we can take this up to release so it will ride along with the 27.0.1 dot release going to build today.
Attachment #8346300 - Flags: approval-mozilla-release?
Attachment #8346300 - Flags: approval-mozilla-release+
Attachment #8346300 - Flags: approval-mozilla-beta?
Attachment #8346300 - Flags: approval-mozilla-beta+
Attachment #8346300 - Flags: approval-mozilla-aurora?
Attachment #8346300 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-release/rev/93a51a9585b0

I'll push this to aurora/beta in a little bit with some other uplifts.
status-b2g-v1.2: --- → unaffected
status-b2g-v1.3: --- → affected
status-b2g-v1.4: --- → fixed
status-firefox27: affected → fixed
status-firefox30: --- → fixed
(Assignee)

Comment 20

4 years ago
The v00 patch may not have been tested with PR_LOGGING undefined, and it's likely that it won't build
with PR_LOGGING undefined because png_chunk_error() gets defined out of pngerror.c.  If that happens, the fix is to use png_error() instead of png_chunk_error().  In libpng-1.6.9 we are using png_error(); see bug #952505.  I think PR_LOGGING is always set (via FORCE_PR_LOGGING) in the public Firefox releases, so this problem may not happen.
Is it worth adding a test for this?
Flags: in-testsuite?
(Assignee)

Comment 22

4 years ago
Created attachment 8374962 [details]
PNG for test case: 64x32 indexed with zero-length palette.

zero-plte-ref.png for testcase
(Assignee)

Comment 23

4 years ago
Created attachment 8375006 [details] [diff] [review]
t00-945912-crashtest.diff

The t00 patch adds a crashtest for this bug.
status-b2g-v1.3: affected → fixed
(Assignee)

Updated

4 years ago
Blocks: 952505
No longer depends on: 952505
I reproduce the crash using the attached corrupted png file on Mac OS X 10.8.5 and Win 7 x64
Using the FF 27 release build id : 20140127194636 .

The bug was verified using the following environment:

FF 27.0.1 
Build Id : 20140212131424
OS: Win 7 x64, Mac Os x 10.8.5 and Ubuntu 12.04 x32
status-firefox27: fixed → verified
Comment on attachment 8375006 [details] [diff] [review]
t00-945912-crashtest.diff

Confirmed that this crashes pre-fix and doesn't afterwards :)
Attachment #8375006 - Flags: review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c1080ed8c03

Thanks, Glenn!
Flags: in-testsuite? → in-testsuite+
I reproduce the crash using the attached corrupted png file on Mac OS X 10.8.5 and Win 7 x64
Using the FF 27 release build id : 20140127194636 .

The bug was verified using the following environment:

FF 28.0b3
Build Id : 20140213172947
OS: Win 7 x64, Mac Os x 10.8.5 and Ubuntu 13.04 x32
status-firefox28: fixed → verified
Whiteboard: [qa-]
status-b2g-v1.3T: --- → fixed
You need to log in before you can comment on or make changes to this bug.