New issue
Advanced search Search tips

Issue 838672 link

Starred by 2 users

WebRTC: Out-of-bounds memory access in WebRTC VP9 Missing Frame Processing

Project Member Reported by natashenka@google.com, May 1

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::MissingRequiredFrameVp9 contains the following code:

  size_t temporal_idx = info.gof->temporal_idx[gof_idx];
  ...
  for (size_t l = 0; l < temporal_idx; ++l) {
      ...
      auto missing_frame_it = missing_frames_for_layer_[l].lower_bound(ref_pid);

missing_frames_for_layer_ is a std::array of length kMaxTemporalLayers which equals 5.

Meanwhile, values in the temporal_idx array are read in rtp_format_vp9.cc in the following code:

    RETURN_FALSE_ON_ERROR(parser->ReadBits(&t, 3));
    ...
    vp9->gof.temporal_idx[i] = t;

Reading three bits makes the maximum size of temporal_idx 7, which can go out of bounds of the missing_frames_for_layer_ array.

This issue causes a crash in 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

The attached file 'missingframe' contains the VP9 frame that causes this crash.

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.

Please note it is not sufficient to fix this issue in Chrome, it needs to be upstreamed to webrtc, so all users of the library can get the fix. 

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.
 
webrtc-from-chat.zip
396 KB Download
missingframe
6.6 KB View Download
webrtcserver.py
3.7 KB View Download
rtp_format_vp9.cc
32.8 KB View Download
Labels: Security_Severity-High M-67 Security_Impact-Stable Pri-1
Owner: philipel@chromium.org
Status: Assigned (was: Unconfirmed)
Cc: ssilkin@chromium.org
Labels: OS-Android OS-Chrome OS-Fuchsia OS-iOS OS-Linux OS-Mac OS-Windows
Thank you for the detailed bug report!

I could not get your reproduction steps to work, but I have uploaded a CL (https://webrtc-review.googlesource.com/c/src/+/73701) that I hope you could test.

You can run "git cl patch 73701" to apply the patch locally.
Status: Fixed (was: Assigned)
This fixes it, thank you!
Project Member

Comment 5 by bugdroid1@chromium.org, May 3

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/a157e080931581b5f6f3f9bc580a137e6062e45b

commit a157e080931581b5f6f3f9bc580a137e6062e45b
Author: philipel <philipel@webrtc.org>
Date: Thu May 03 12:00:41 2018

VP9 temporal index bounds check.

Bug:  chromium:838672 
Change-Id: Ia531327858b6e40cb7fa03ca1b98c120ba4e1389
Reviewed-on: https://webrtc-review.googlesource.com/73701
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23099}
[modify] https://crrev.com/a157e080931581b5f6f3f9bc580a137e6062e45b/modules/video_coding/rtp_frame_reference_finder.cc
[modify] https://crrev.com/a157e080931581b5f6f3f9bc580a137e6062e45b/modules/video_coding/rtp_frame_reference_finder_unittest.cc

Project Member

Comment 6 by sheriffbot@chromium.org, May 3

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 7 by bugdroid1@chromium.org, May 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/213a35ad18a045cba0b1edb22f33dfc8d91c8681

commit 213a35ad18a045cba0b1edb22f33dfc8d91c8681
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Thu May 03 17:33:07 2018

Roll src/third_party/webrtc/ 01042067b..a1f566b28 (14 commits)

https://webrtc.googlesource.com/src.git/+log/01042067beb1..a1f566b28a69

$ git log 01042067b..a1f566b28 --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG= chromium:838672 ,chromium:None,chromium:755920


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: I2bfdb1d8fa2b497d772d58cf21d7e3c1d1a67c5f
Reviewed-on: https://chromium-review.googlesource.com/1042493
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@{#555790}
[modify] https://crrev.com/213a35ad18a045cba0b1edb22f33dfc8d91c8681/DEPS

Labels: Merge-Request-67
Project Member

Comment 9 by sheriffbot@chromium.org, May 4

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: Less than 21 days to go before AppStore submit on M67
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
Cc: awhalley@chromium.org
+awhalley@ (Security TPM) for M67 merge review.
NextAction: 2018-05-07
The NextAction date has arrived: 2018-05-07
Cc: terelius@chromium.org tommi@chromium.org
govind@ - good for 67
Labels: -Merge-Review-67 Merge-Approved-67
Approving merge to M67 branch 3396 based on comment #14. Please merge ASAP so we can pick it up for this week Beta release. Thank you.
Labels: -Merge-Approved-67 Merge-Merged
 philipel@, could you pls provide M67 merged cl link here?
Labels: -Merge-Merged merge-merged-67
Thank you  philipel@.
Cc: huib@chromium.org
Labels: Release-0-M67
Labels: CVE-2018-6129 CVE_description-missing
Project Member

Comment 23 by sheriffbot@chromium.org, Aug 9

Labels: -Restrict-View-SecurityNotify allpublic
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