1429216 - UAF due to webrtc codec init failure

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P1)

Description

[Randell Jesup [:jesup] (needinfo me)]

+++ This bug was initially created as a clone of Bug #1426449 +++

If InitEncode() fails, in non-debug builds, a race is set up where a pending OnBitrateUpdated will try to access the temporal_layers_, which failure of SetSendCodec() will cause to be deleted before OnBitrateUpdated runs.  This causes a UAF.

Also, generally clean up and exit on failure.

InitEncode shouldn't fail -- but if it does (and there are a lot of ways it can), we should handle it smoothly.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: handle RegisterSendCodec() failure better in non-debug builds

works fine if I force the InitEncode to fail now

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Attachment: handle RegisterSendCodec() failure better in non-debug builds

forgot to include the .h file

Comment 3

[Nils Ohlmeier [:drno]]

Comment on attachment 8941201 [details] [diff] [review]
handle RegisterSendCodec() failure better in non-debug builds

Review of attachment 8941201 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

Comment 4

[Nils Ohlmeier [:drno]]

Randell is this something we would want to upstream?

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Yes, though there are probably cleaner ways to do with (with larger changes), so they may not want to take it as-is - but that's up to them; we should put it up.

Note: this is a sec issue.  We should try to coordinate this with Google, though they don't seem to handle sec issues in the webrtc code in the same way we do.

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Sec and beta requests are in bug 1426449

Comment 7

[Dan Minor [:dminor]]

Upstream bug filed: https://bugs.chromium.org/p/webrtc/issues/detail?id=8754