Closed Bug 1436117 (CVE-2018-5160) Opened 2 years ago Closed 2 years ago

A WrappedI420Buffer in MediaPipeline might outlive its buffer

Categories

(Core :: WebRTC: Audio/Video, defect, P2)

60 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- fixed

People

(Reporter: pehrsons, Assigned: pehrsons)

References

Details

Attachments

(1 file)

One use of a WrappedI420Buffer in MediaPipeline [1] gets passed on further into the stack, but nothing keeps the PlanarYCbCrImage that owns the actual pixel buffer alive.

The WrappedI420Buffer outliving the PlanarYCbCrImage would be bad, but I hope and think we only end up reading that data as an image in a webrtc encoder and not doing any writing.

[1] https://searchfox.org/mozilla-central/rev/f80722d4f3bfb722c5ec53880c4a7efb71285676/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp#345-356
Rank: 15
Priority: -- → P2
Comment on attachment 8953919 [details]
Bug 1436117 - Keep aImage around until the WrappedI420Buffer is released.

https://reviewboard.mozilla.org/r/223070/#review229072

lgtm
Attachment #8953919 - Flags: review?(dminor) → review+
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ca97ef07a045
Keep aImage around until the WrappedI420Buffer is released. r=dminor
https://hg.mozilla.org/mozilla-central/rev/ca97ef07a045
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Comment on attachment 8953919 [details]
Bug 1436117 - Keep aImage around until the WrappedI420Buffer is released.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1341285
[User impact if declined]: There's a risk of the webrtc encoder accessing uninitialized memory. However we haven't seen or heard reports about this so I gather it's a very low risk.
[Is this code covered by automated tests?]: Yes, though not explicitly testing what this fix is for, as it is speculative.
[Has the fix been verified in Nightly?]: It's speculative, so no
[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?]: No
[Why is the change risky/not risky?]: It may extend the lifetime of some internal images. The worst that could happen would be shutdown leaks.
[String changes made/needed]: None
Attachment #8953919 - Flags: approval-mozilla-beta?
Comment on attachment 8953919 [details]
Bug 1436117 - Keep aImage around until the WrappedI420Buffer is released.

I'd like to let this ride with 60 unless you feel strongly about fixing it in 59.
Attachment #8953919 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Alias: CVE-2018-5160
Duplicate of this bug: 1453899
Duplicate of this bug: 1429617
You need to log in before you can comment on or make changes to this bug.