Closed
Bug 945912
Opened 12 years ago
Closed 11 years ago
Crash [@ MOZ_PNG_do_expand_plte] with broken png file
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla30
Tracking | Status | |
---|---|---|
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 |
People
(Reporter: decoder, Assigned: glennrp+bmo)
References
Details
(Keywords: crash, testcase, Whiteboard: [qa-])
Attachments
(4 files)
518 bytes,
image/png
|
Details | |
1023 bytes,
patch
|
jrmuizel
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
lsblakk
:
approval-mozilla-release+
|
Details | Diff | Splinter Review |
82 bytes,
image/png
|
Details | |
959 bytes,
patch
|
RyanVM
:
review+
|
Details | Diff | Splinter Review |
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
[...]
Updated•12 years ago
|
Severity: major → critical
Comment 1•12 years ago
|
||
crashes on same address on windows: bp-2084589a-45fa-48cf-87a7-27de52131211
Comment 2•12 years ago
|
||
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•12 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•12 years ago
|
Assignee: nobody → glennrp+bmo
OS: Linux → All
Hardware: x86_64 → All
Assignee | ||
Updated•12 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•12 years ago
|
||
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•12 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: → 841734
Updated•12 years ago
|
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•12 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•12 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•12 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 | ||
Comment 9•11 years ago
|
||
Needs try server run. If applied, this will bit rot the libpng-1.6.8 patch slightly.
Flags: needinfo?(ryanvm)
Comment 10•11 years ago
|
||
Flags: needinfo?(ryanvm)
Assignee | ||
Comment 11•11 years ago
|
||
try server results look pretty good, all green except for one timeout that I can't tell was related to this patch.
Comment 12•11 years ago
|
||
Not at all.
Assignee | ||
Comment 13•11 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)
Updated•11 years ago
|
Attachment #8346300 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 14•11 years ago
|
||
Keywords: checkin-needed
Comment 15•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e96704e1a7ea
Please nominate this for uplift to the affected branches.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Assignee | ||
Comment 16•11 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 17•11 years ago
|
||
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+
Comment 18•11 years ago
|
||
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-firefox30:
--- → fixed
Comment 19•11 years ago
|
||
Assignee | ||
Comment 20•11 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.
Assignee | ||
Comment 22•11 years ago
|
||
zero-plte-ref.png for testcase
Assignee | ||
Comment 23•11 years ago
|
||
The t00 patch adds a crashtest for this bug.
Comment 24•11 years ago
|
||
Updated•11 years ago
|
Assignee | ||
Updated•11 years ago
|
Comment 25•11 years ago
|
||
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
Comment 26•11 years ago
|
||
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+
Comment 27•11 years ago
|
||
Flags: in-testsuite? → in-testsuite+
Comment 28•11 years ago
|
||
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
Comment 29•11 years ago
|
||
Comment 30•11 years ago
|
||
Comment 31•11 years ago
|
||
Updated•11 years ago
|
status-b2g-v1.3T:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•