OOM problems in nsPNGEncoder.cpp

RESOLVED FIXED in mozilla10

Status

()

Core
ImageLib
RESOLVED FIXED
10 years ago
6 years ago

People

(Reporter: mats, Assigned: bjacob)

Tracking

Trunk
mozilla10
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
A couple of places we should check if 'new' fails:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/modules/libpr0n/encoders/png/nsPNGEncoder.cpp&rev=1.5&root=/cvsroot&mark=266,257#255

I think the error handling here is insufficient:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/modules/libpr0n/encoders/png/nsPNGEncoder.cpp&rev=1.5&root=/cvsroot&mark=574#561

If we leave 'that->mImageBuffer' non-null I think other code (the destructor,
Close()) might free it twice, and other methods use "mImageBuffer != nsnull"
to check if it's valid...
We should set it to nsnull after PR_Free?
(Assignee)

Comment 1

6 years ago
operator new is now explicitly infallible,

  https://developer.mozilla.org/en/Infallible_memory_allocation

so the first issue here is fixed.

The second issue seems to still be a real possible bug, so let me make a 1-line patch...
(Assignee)

Comment 2

6 years ago
Created attachment 565621 [details] [diff] [review]
null the imagebuffer pointer then we free it

Brian seems to be the last to have touched this file -> asking for review.
Attachment #565621 - Flags: review?(netzen)
Comment on attachment 565621 [details] [diff] [review]
null the imagebuffer pointer then we free it

Looks good.
Attachment #565621 - Flags: review?(netzen) → review+
(Assignee)

Comment 4

6 years ago
Maybe we need to do a post-mortem here. This bug had 'OOM' in its title. OOM bugs are probably a lot more likely than the average to be security sensitive. Can we have automatic queries to make sure that at least bugs explicitly mentioning OOM issues don't fall off the radar?
(Assignee)

Comment 5

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/b79549daab38

Comment 6

6 years ago
https://hg.mozilla.org/mozilla-central/rev/b79549daab38
Assignee: nobody → bjacob
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
You need to log in before you can comment on or make changes to this bug.