Closed
Bug 681190
Opened 13 years ago
Closed 13 years ago
crash nsPNGEncoder::ConvertHostARGBRow
Categories
(Core :: Graphics: ImageLib, defect)
Tracking
()
RESOLVED
FIXED
mozilla9
People
(Reporter: m_kato, Assigned: arno)
References
Details
(Keywords: crash)
Crash Data
Attachments
(2 files, 1 obsolete file)
195 bytes,
text/html
|
Details | |
4.22 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-a066da9e-5773-4da0-b807-07ba32110822 . ============================================================= This is crash sample. Repro rate is 100% <!DOCTYPE html> <html> <body> <canvas id="crashtest" width="50000" height="50000" /> <script> var file = document.getElementById("crashtest").mozGetAsFile(""); </script> </body> </html>
Reporter | ||
Updated•13 years ago
|
Component: General → ImageLib
Product: Firefox → Core
QA Contact: general → imagelib
Assignee | ||
Comment 2•13 years ago
|
||
Crash happens, when emptyCanvas is created in nsHTMLCanvasElement::ExtractData, it's too big so it's data is leaved to null. Then, in nsPNGEncoder::ConvertHostARGBRow, const PRUint32& pixelIn = ((const PRUint32*)(aSrc))[x]; segfaults (aSrc is emptyCanvas data). Here is a patch proposal. It does two things to ensure crash does not happen: First, return early from nsHTMLCanvasElement::ExtractData in case emptyCanvas data is null. Also, it returns early from nsJPEGEncoder::InitFromData and nsPNGEncoder::InitFromData in case they receive a null data argument. That should prevent some similar crashes in the future.
Attachment #555401 -
Flags: review?(roc)
Assignee | ||
Comment 3•13 years ago
|
||
Here is try server result for the patch: http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=64ff7c725a07
Comment on attachment 555401 [details] [diff] [review] patch proposal Review of attachment 555401 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/html/content/src/nsHTMLCanvasElement.cpp @@ +228,5 @@ > nsIntSize size = GetWidthHeight(); > if (!mCurrentContext) { > emptyCanvas = new gfxImageSurface(gfxIntSize(size.width, size.height), gfxASurface::ImageFormatARGB32); > + NS_ENSURE_TRUE(emptyCanvas, NS_ERROR_OUT_OF_MEMORY); > + NS_ENSURE_TRUE(emptyCanvas->Data(), NS_ERROR_INVALID_ARG); emptyCanvas can't be null because the new is infallible. instead of checking emptyCanvas->Data(), check emptyCanvas->CairoStatus().
Assignee | ||
Comment 5•13 years ago
|
||
updated patch
Attachment #555401 -
Attachment is obsolete: true
Attachment #555401 -
Flags: review?(roc)
Attachment #555726 -
Flags: review?(roc)
Attachment #555726 -
Flags: review?(roc) → review+
Assignee | ||
Updated•13 years ago
|
Keywords: checkin-needed
Comment 6•13 years ago
|
||
In my queue, pushing to inbound once confirmed everything builds locally.
Status: NEW → ASSIGNED
Keywords: checkin-needed
Comment 7•13 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/2da2cc9a885c
Target Milestone: --- → mozilla9
Comment 8•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/2da2cc9a885c
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•