Closed
Bug 681190
Opened 14 years ago
Closed 14 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•14 years ago
|
Component: General → ImageLib
Product: Firefox → Core
QA Contact: general → imagelib
|
Assignee | |
Comment 2•14 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•14 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•14 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•14 years ago
|
Keywords: checkin-needed
|
||
Comment 6•14 years ago
|
||
In my queue, pushing to inbound once confirmed everything builds locally.
Status: NEW → ASSIGNED
Keywords: checkin-needed
|
|
||
Comment 7•14 years ago
|
||
Target Milestone: --- → mozilla9
|
||
Comment 8•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•