Issue metadata
Sign in to add a comment
|
Security: WebRTC: Out-of-bounds memory access in WebRTC VP9 Frame Processing |
||||||||||||||||||||||
Issue description
There is a missing check in VP9 frame processing that could lead to memory corruption.
In the file video_coding/rtp_frame_reference_finder.cc, the function RtpFrameReferenceFinder::ManageFrameVp9 fetches the GofInfo based on a pic_idx parsed from the incoming packet header. If the incoming frame is of type kVideoFrameKey, find is called on an iterator and the result is used without checking whether the it succeeds.
if (frame->frame_type() == kVideoFrameKey) {
...
GofInfo info = gof_info_.find(codec_header.tl0_pic_idx)->second;
FrameReceivedVp9(frame->id.picture_id, &info);
UnwrapPictureIds(frame);
return kHandOff;
}
This can cause a pointer to memory outside the gof_info_ map to be passed to FrameReceivedVp9. This function both reads and writes the info structure.
This issue does not crash reliably, so I recommend reproducing it using an asan build of Chrome. To reproduce the issue:
1) unzip the attached webrtc-from-chat.zip on a local webserver
2) fetch the webrtc source (https://webrtc.org/native-code/development/), and replace src/modules/rtp_rtcp/source/rtp_format_vp9.cc with the version attached to the code
3) build webrtc, including the examples
4) run the attached webrtcserver.py with python 3.6 or higher
5) start the peerconnection_client sample in the webrtc examples. Connect to the recommended server, and then select test2 as the peer to connect to
6) visit http://127.0.0.1/webrtc-from-chat/index.html in chrome
7) Enter any username and hit "Log in"
8) Type anything into the chat window at the bottom and hit send
Chrome should crash in a few seconds.
Though the attached PoC requires user interaction, it is not necessary to exercise this issue in a browser.
This issue affects any browser that supports VP9, and can be reached by loading a single webpage (though some browsers will prompt for permissions). It also affects native clients (such as mobile applications) that use webrtc and support VP9, though the user has to place or answer a video call for their client to be in the state where this issue is reachable.
I recommend fixing this by changing the above code to:
auto gof_info_it = gof_info_.find(codec_header.tl0_pic_idx);
if (gof_info_it == gof_info_.end())
return kDrop;
GofInfo info = gof_info_it->second;
FrameReceivedVp9(frame->id.picture_id, &info);
I have verified that this fix would prevent the crash.
Please note that is is not sufficient to fix this bug in Chrome, it needs to be fixed in WebRTC upstream as well to be fixed for all affected users.
This bug is subject to a 90 day disclosure deadline. After 90 days elapse
or a patch has been made broadly available, the bug report will become
visible to the public.
,
May 1 2018
,
May 2 2018
,
May 2 2018
,
May 2 2018
I believe this problem should be fixed by https://webrtc-review.googlesource.com/c/src/+/60260 Unfortunately I could not get your repro steps to work, could you test this CL? You can run "git cl patch 60260" to apply the patch locally.
,
May 2 2018
,
May 2 2018
Yes, this resolves it. Thank you!
,
May 3 2018
,
May 4 2018
,
May 4 2018
This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 4 2018
+awhalley@ (Security TPM) for M67 merge review.
,
May 4 2018
,
May 7 2018
The NextAction date has arrived: 2018-05-07
,
May 7 2018
The following revision refers to this bug: https://webrtc.googlesource.com/src.git/+/156436056ec1d9cd56ae3d7bce29b356b404f9ca commit 156436056ec1d9cd56ae3d7bce29b356b404f9ca Author: philipel <philipel@webrtc.org> Date: Mon May 07 11:29:44 2018 VP9 RTP frame reference finder cleanup. Bug: chromium:838402 Change-Id: Ib6117db5aeb83fc03f72759be2e42d0a4ceb94b6 Reviewed-on: https://webrtc-review.googlesource.com/60260 Reviewed-by: Stefan Holmer <stefan@webrtc.org> Reviewed-by: Niels Moller <nisse@webrtc.org> Commit-Queue: Philip Eliasson <philipel@webrtc.org> Cr-Commit-Position: refs/heads/master@{#23139} [modify] https://crrev.com/156436056ec1d9cd56ae3d7bce29b356b404f9ca/modules/video_coding/rtp_frame_reference_finder.cc
,
May 7 2018
,
May 7 2018
philipel@, could you pls clarify which CL you're requesting M67 merge for? CL listed at #14 landed in trunk just 4 hrs back so not in canary yet.
,
May 7 2018
The merge request is for the CL in #14. I realize that CL is a bit tricky to review since it is more than just a security fix, so what I could do instead is to implement the solution suggested by natashenka@. Then the CL would be trivial.
,
May 7 2018
Thank you philipel@. Pls update the bug with canary result for cl listed at #14. awhalley@, PTAL comment #17. Thank you.
,
May 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/71098015063adea710cce4616686d08d65fc1940 commit 71098015063adea710cce4616686d08d65fc1940 Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Date: Mon May 07 20:43:08 2018 Roll src/third_party/webrtc/ 823f9135f..d5ef6ff25 (15 commits) https://webrtc.googlesource.com/src.git/+log/823f9135f858..d5ef6ff258da $ git log 823f9135f..d5ef6ff25 --date=short --no-merges --format='%ad %ae %s' Created with: roll-dep src/third_party/webrtc BUG=chromium:None,chromium:None,chromium:840347,chromium:839860,chromium:838402 The AutoRoll server is located here: https://webrtc-chromium-roll.skia.org Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng TBR=webrtc-chromium-sheriffs-robots@google.com Change-Id: Ibaedd51469f75941f90aed9f4ba6bb686cd79f6c Reviewed-on: https://chromium-review.googlesource.com/1048112 Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#556561} [modify] https://crrev.com/71098015063adea710cce4616686d08d65fc1940/DEPS
,
May 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d1429ef7c2699d69eec780f6f8beba5ecab9e312 commit d1429ef7c2699d69eec780f6f8beba5ecab9e312 Author: Kunihiko Sakamoto <ksakamoto@chromium.org> Date: Tue May 08 03:27:37 2018 Revert "Roll src/third_party/webrtc/ 823f9135f..d5ef6ff25 (15 commits)" This reverts commit 71098015063adea710cce4616686d08d65fc1940. Reason for revert: ios build is still broken. https://ci.chromium.org/buildbot/chromium.mac/ios-device-xcode-clang/59679 Original change's description: > Roll src/third_party/webrtc/ 823f9135f..d5ef6ff25 (15 commits) > > https://webrtc.googlesource.com/src.git/+log/823f9135f858..d5ef6ff258da > > $ git log 823f9135f..d5ef6ff25 --date=short --no-merges --format='%ad %ae %s' > > Created with: > roll-dep src/third_party/webrtc > BUG=chromium:None,chromium:None,chromium:840347,chromium:839860,chromium:838402 > > > The AutoRoll server is located here: https://webrtc-chromium-roll.skia.org > > Documentation for the AutoRoller is here: > https://skia.googlesource.com/buildbot/+/master/autoroll/README.md > > If the roll is causing failures, please contact the current sheriff, who should > be CC'd on the roll, and stop the roller if necessary. > > > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng > TBR=webrtc-chromium-sheriffs-robots@google.com > > Change-Id: Ibaedd51469f75941f90aed9f4ba6bb686cd79f6c > Reviewed-on: https://chromium-review.googlesource.com/1048112 > Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> > Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> > Cr-Commit-Position: refs/heads/master@{#556561} TBR=webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com,webrtc-chromium-sheriffs-robots@google.com Change-Id: I2823387c7aadad799befbf88cc5b4503a379dd3c No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:None, chromium:840347, chromium:839860, chromium:838402 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng Reviewed-on: https://chromium-review.googlesource.com/1049345 Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org> Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org> Cr-Commit-Position: refs/heads/master@{#556675} [modify] https://crrev.com/d1429ef7c2699d69eec780f6f8beba5ecab9e312/DEPS
,
May 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ebc7dc2c2cccb2e04ed594fa68c0a1fb0d7a59e6 commit ebc7dc2c2cccb2e04ed594fa68c0a1fb0d7a59e6 Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Date: Tue May 08 14:16:42 2018 Roll src/third_party/webrtc/ 823f9135f..826738b78 (29 commits) https://webrtc.googlesource.com/src.git/+log/823f9135f858..826738b78c6a $ git log 823f9135f..826738b78 --date=short --no-merges --format='%ad %ae %s' Created with: roll-dep src/third_party/webrtc BUG=chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:840347,chromium:839860,chromium:838402 The AutoRoll server is located here: https://webrtc-chromium-roll.skia.org Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng TBR=webrtc-chromium-sheriffs-robots@google.com Change-Id: I555e922aefb9dac0e262c32fe8562a47f7a15144 Reviewed-on: https://chromium-review.googlesource.com/1049945 Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#556789} [modify] https://crrev.com/ebc7dc2c2cccb2e04ed594fa68c0a1fb0d7a59e6/DEPS
,
May 8 2018
This CL https://webrtc-review.googlesource.com/c/src/+/74923 is intended to be merged to M67. It is only a small part of the CL that landed on master (https://webrtc-review.googlesource.com/c/src/+/60260) that fixed this problem. We are waiting for the CL on master to reach canary before we land the CL in 67.
,
May 8 2018
Pls update the bug with canary result.
,
May 8 2018
,
May 9 2018
The NextAction date has arrived: 2018-05-09
,
May 9 2018
Don't really have an update yet. Still waiting for the CL (https://chromiumdash.appspot.com/commit/156436056ec1d9cd56ae3d7bce29b356b404f9ca) to reach canary.
,
May 9 2018
Pls update the bug with canary result tomorrow. Thank you.
,
May 9 2018
I will be OoO tomorrow but back on friday, I will update the bug then.
,
May 11 2018
The NextAction date has arrived: 2018-05-11
,
May 11 2018
The CL was first part of 68.0.3425.0, which was released 9th of May on android/mac/windows (not linux). Canary results look good.
,
May 11 2018
thanks philipel@ govind@ - good for 67
,
May 11 2018
Approving merge to M67 branch 3396 based on comment #30 and #31. Assuming only cl listed at #14 need a merge. Pls merge ASAP. Thank you.
,
May 11 2018
This CL has now been merged: https://webrtc-review.googlesource.com/c/src/+/74923
,
May 11 2018
,
May 11 2018
Reverted the merge since it broke the build.
,
May 11 2018
Revert CL here: https://webrtc-review.googlesource.com/c/src/+/76180
,
May 14 2018
Sorry for messing up the merge :( The updated CL has now landed: https://webrtc-review.googlesource.com/c/src/+/76480
,
May 28 2018
,
May 29 2018
,
May 29 2018
,
May 30 2018
,
Aug 9
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by elawrence@chromium.org
, May 1 2018Components: -Blink>WebRTC Blink>WebRTC>Video
Owner: philipel@chromium.org