Closed
Bug 1323877
Opened 8 years ago
Closed 7 years ago
allow multipart images to receive no OnDataAvailable calls
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla53
People
(Reporter: tnikkel, Assigned: tnikkel)
References
Details
(Keywords: sec-audit, Whiteboard: [post-critsmash-triage][adv-main52-])
Attachments
(1 file)
4.06 KB,
patch
|
aosmond
:
review+
jcristau
:
approval-mozilla-aurora+
dveditz
:
sec-approval+
|
Details | Diff | Splinter Review |
Bug 1320899 exposed some problems in the networking code with multipart images. Sometimes it would fail to send an OnDataAvailable call. I'm not sure if it is guaranteed that we will always get an OnDataAvailable call (say if the part had 0 bytes) so we should make imagelib deal with that case. Currently imagelib gets confused if this happens. Filing as security to be safe, I haven't thought through if it actually is.
Assignee | ||
Comment 1•8 years ago
|
||
Assignee: nobody → tnikkel
Assignee | ||
Updated•8 years ago
|
Attachment #8819111 -
Flags: review?(aosmond)
Updated•8 years ago
|
Attachment #8819111 -
Flags: review?(aosmond) → review+
Updated•8 years ago
|
Group: core-security → gfx-core-security
Assignee | ||
Comment 2•7 years ago
|
||
Comment on attachment 8819111 [details] [diff] [review] patch [Security approval request comment] How easily could an exploit be constructed based on the patch? I'm not sure if this is a security issue or not. It will cause an assert, but I'm not sure if things can get worse. It would take a lot of thinking to figure out what the exact impact is. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? The code comment describes the problem pretty clearly. I could leave it out and land later. Which older supported branches are affected by this flaw? All. If not all supported branches, which bug introduced the flaw? Bug 1112972 rewrote most of the multipart image code. The previous code could have had the same bug though. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? trivial How likely is this patch to cause regressions; how much testing does it need? should be fairly safe, some time baking would be good
Attachment #8819111 -
Flags: sec-approval?
Comment 3•7 years ago
|
||
Comment on attachment 8819111 [details] [diff] [review] patch we may want to uplift this, to aurora at least, just to be safe.
Attachment #8819111 -
Flags: sec-approval? → sec-approval+
Assignee | ||
Comment 4•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4f5ced881d5988c73654823e85b07083e1144ded
Comment 5•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4f5ced881d59
Target Milestone: --- → mozilla53
Comment 6•7 years ago
|
||
[Tracking Requested - why for this release]: per dan's comment in #3
tracking-firefox52:
--- → ?
Assignee | ||
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 8•7 years ago
|
||
Comment on attachment 8819111 [details] [diff] [review] patch Approval Request Comment [Feature/Bug causing the regression]: Bug 1112972 rewrote most of the multipart image code. The previous code could have had the same bug though. [User impact if declined]: speculative security fix [Is this code covered by automated tests?]: not for this specific issue, no [Has the fix been verified in Nightly?]: can't really verify it, speculative issue [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: should be pretty safe [Why is the change risky/not risky?]: we will just create an empty image if there is no data for one part of a multipart image [String changes made/needed]: none
Attachment #8819111 -
Flags: approval-mozilla-aurora?
Comment 9•7 years ago
|
||
Comment on attachment 8819111 [details] [diff] [review] patch handle empty parts in multipart images, aurora52+
Attachment #8819111 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 10•7 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/995ecbd221ea
status-firefox52:
--- → fixed
Updated•7 years ago
|
Group: gfx-core-security → core-security-release
Updated•7 years ago
|
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Updated•7 years ago
|
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main52-]
Updated•7 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•