Closed
Bug 1315288
Opened 8 years ago
Closed 8 years ago
Crash in memcpy | copy_and_extend_plane
Categories
(Core :: WebRTC, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla52
People
(Reporter: noni, Assigned: jesup)
References
(Blocks 1 open bug)
Details
(4 keywords)
Crash Data
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
jesup
:
review+
abillings
:
approval-mozilla-aurora+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is
report bp-411947f7-dcdf-4353-bfb7-415742161027.
=============================================================
Ran into this issue twice while testing Webrtc calls. Nothing special done when running into this, just entered the call and crashed after a few moments.
Tested with https://apprtc.appspot.com/ on Windows 7x64
Second crash report: bp-8574ae8b-5f1d-4207-b7f7-2e8de2161027
Reporter | ||
Updated•8 years ago
|
Assignee | ||
Comment 1•8 years ago
|
||
This is almost certainly the VP9 version of bug 1263384 (which fixed it for VP8, and was fixed in Fx47). Since we stil haven't updated from upstream since then, we need to mirror the patch to VP9.
This is a write-past-end-of-allocation bug. Note that VP9 is enabled for webrtc in 51, though not the default codec.
Assignee: nobody → rjesup
Group: media-core-security
Status: NEW → ASSIGNED
Rank: 10
status-firefox50:
--- → unaffected
status-firefox52:
--- → affected
Keywords: csectype-bounds,
sec-high
Priority: -- → P1
Assignee | ||
Comment 2•8 years ago
|
||
Attachment #8808214 -
Flags: review?(giles)
Comment 3•8 years ago
|
||
Comment on attachment 8808214 [details] [diff] [review]
Add input checks for VP9
Review of attachment 8808214 [details] [diff] [review]:
-----------------------------------------------------------------
Same fix, ok. Please add it to input_frame_validation.patch as well so it gets checked on update.
Attachment #8808214 -
Flags: review?(giles) → review+
Assignee | ||
Comment 4•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Attachment #8808214 -
Attachment is obsolete: true
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 8808314 [details] [diff] [review]
Add input checks for VP9
Carry forward r+. Updates the update script as well; adds this as a separate patch instead of updating the original patch.
[Security approval request comment]
How easily could an exploit be constructed based on the patch? Pretty hard, you'd have to figure out how to get it to generate the "wrong" size frame and control the data in that frame. This perhaps could be done with a canvas capture source, but provoking the race condition might be tricky. Then you have a bounds violation with controllable data, so you'd have to turn that into an exploit in some manner.
Overall, possible, but not simple. And not very obvious from the patch.
Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. They do make it clear it's an input-validation issue. This exact same patch for VP8 landed and uplifted to 47, and has already been opened.
Which older supported branches are affected by this flaw? 51 and 52 only.
If not all supported branches, which bug introduced the flaw? Enabling VP9 by default in 51.
Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Trivial, applies directly.
How likely is this patch to cause regressions; how much testing does it need? Virtually no chance of regression.
Attachment #8808314 -
Flags: sec-approval?
Attachment #8808314 -
Flags: review+
Attachment #8808314 -
Flags: approval-mozilla-aurora?
Updated•8 years ago
|
Keywords: regression
Assignee | ||
Comment 7•8 years ago
|
||
Not a regression per-se, the bug in VP9 has always been there but was only hittable if you set a pref to enable VP9 in webrtc.
Keywords: regression
Comment 8•8 years ago
|
||
Comment on attachment 8808314 [details] [diff] [review]
Add input checks for VP9
Sec-approval+ and I'm approving Aurora too since 50 is unaffected. Let's get this fixed!
Attachment #8808314 -
Flags: sec-approval?
Attachment #8808314 -
Flags: sec-approval+
Attachment #8808314 -
Flags: approval-mozilla-aurora?
Attachment #8808314 -
Flags: approval-mozilla-aurora+
Updated•8 years ago
|
Comment 9•8 years ago
|
||
Comment 10•8 years ago
|
||
jesup: shall we land this on trunk or do you want to land this on inbound ?
Flags: needinfo?(rjesup)
Assignee | ||
Comment 11•8 years ago
|
||
I'm good with landing directly on central; inbound would be ok too since I'm sure it'll pick up a merge before uplift
Flags: needinfo?(rjesup)
Assignee | ||
Comment 12•8 years ago
|
||
Comment 13•8 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Updated•8 years ago
|
Group: media-core-security → core-security-release
Updated•8 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•