New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 840536 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: WebRTC: Type Confusion when processing H264 NAL packet

Project Member Reported by natashenka@google.com, May 7 2018

Issue description

Type confusion can occur when processing a H264 packet. In the method PacketBuffer::FindFrames in modules/video_coding/packet_buffer.cc there is a loop on line 296 that goes through the data_buffer_ vector backwards. The flag is_h264 is set before this loop, and if it is true, the loop extracts and sets h264 struct specific data in each packet of the buffer. This flag is not updated for each packet. So if a number of non-h264 packets are followed by a h264 packet, a VP8 or VP9 packet can be treated at a h264 check, allowing several bounds checks to be bypassed.

 #0 0x55bd530e0721 in webrtc::video_coding::PacketBuffer::FindFrames(unsigned short) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/modules/video_coding/packet_buffer.cc:314:33
    #1 0x55bd530dcb24 in webrtc::video_coding::PacketBuffer::InsertPacket(webrtc::VCMPacket*) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/modules/video_coding/packet_buffer.cc:126:20
    #2 0x55bd530d0592 in webrtc::RtpVideoStreamReceiver::OnReceivedPayloadData(unsigned char const*, unsigned long, webrtc::WebRtcRTPHeader const*) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/video/rtp_video_stream_receiver.cc:242:19
    #3 0x55bd52f225ed in webrtc::RTPReceiverVideo::ParseRtpPacket(webrtc::WebRtcRTPHeader*, webrtc::PayloadUnion const&, unsigned char const*, unsigned long, long) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc:109:26
    #4 0x55bd52f16fbd in webrtc::RtpReceiverImpl::IncomingRtpPacket(webrtc::RTPHeader const&, unsigned char const*, unsigned long, webrtc::PayloadUnion) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc:192:42
    #5 0x55bd530d144f in webrtc::RtpVideoStreamReceiver::ReceivePacket(unsigned char const*, unsigned long, webrtc::RTPHeader const&) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/video/rtp_video_stream_receiver.cc:410:20
    #6 0x55bd530d1142 in webrtc::RtpVideoStreamReceiver::OnRecoveredPacket(unsigned char const*, unsigned long) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/video/rtp_video_stream_receiver.cc:256:3
    #7 0x55bd530f8098 in webrtc::UlpfecReceiverImpl::ProcessReceivedFec() /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc:239:35
    #8 0x55bd530d3fa9 in webrtc::RtpVideoStreamReceiver::ParseAndHandleEncapsulatingHeader(unsigned char const*, unsigned long, webrtc::RTPHeader const&) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/video/rtp_video_stream_receiver.cc:430:23
    #9 0x55bd530d134b in webrtc::RtpVideoStreamReceiver::ReceivePacket(unsigned char const*, unsigned long, webrtc::RTPHeader const&) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/video/rtp_video_stream_receiver.cc:401:5
    #10 0x55bd530d1fc2 in webrtc::RtpVideoStreamReceiver::OnRtpPacket(webrtc::RtpPacketReceived const&) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/video/rtp_video_stream_receiver.cc:301:3
    #11 0x55bd40d97311 in webrtc::RtpDemuxer::OnRtpPacket(webrtc::RtpPacketReceived const&) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/call/rtp_demuxer.cc:157:11
    #12 0x55bd40d9c1a1 in webrtc::RtpStreamReceiverController::OnRtpPacket(webrtc::RtpPacketReceived const&) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/call/rtp_stream_receiver_controller.cc:55:19
    #13 0x55bd52e3fe39 in webrtc::internal::Call::DeliverRtp(webrtc::MediaType, rtc::CopyOnWriteBuffer, webrtc::PacketTime const&) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/call/call.cc:1292:36
    #14 0x55bd52e407e3 in webrtc::internal::Call::DeliverPacket(webrtc::MediaType, rtc::CopyOnWriteBuffer, webrtc::PacketTime const&) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/call/call.cc:1316:10
    #15 0x55bd5358b001 in cricket::WebRtcVideoChannel::OnPacketReceived(rtc::CopyOnWriteBuffer*, rtc::PacketTime const&) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/media/engine/webrtcvideoengine.cc:1444:26
    #16 0x55bd52deb9e2 in cricket::BaseChannel::ProcessPacket(bool, rtc::CopyOnWriteBuffer const&, rtc::PacketTime const&) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/pc/channel.cc:0:21
    #17 0x55bd52e09370 in rtc::AsyncInvoker::OnMessage(rtc::Message*) /usr/local/google/home/natashenka/chromium/src/out/asan/../../third_party/webrtc/rtc_base/asyncinvoker.cc:45:22
    #18 0x55bd52ca1711 in jingle_glue::JingleThreadWrapper::Dispatch(rtc::Message*) /usr/local/google/home/natashenka/chromium/src/out/asan/../../jingle/glue/thread_wrapper.cc:157:22
    #19 0x55bd52ca29ee in jingle_glue::JingleThreadWrapper::RunTask(int) /usr/local/google/home/natashenka/chromium/src/out/asan/../../jingle/glue/thread_wrapper.cc:279:7
    #20 0x55bd447abaf5 in Run /usr/local/google/home/natashenka/chromium/src/out/asan/../../base/callback.h:96:12
    #21 0x55bd447abaf5 in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) /usr/local/google/home/natashenka/chromium/src/out/asan/../../base/debug/task_annotator.cc:101:0
    #22 0x55bd44809665 in base::MessageLoop::RunTask(base::PendingTask*) /usr/local/google/home/natashenka/chromium/src/out/asan/../../base/message_loop/message_loop.cc:319:25
    #23 0x55bd4480a8d4 in DeferOrRunPendingTask /usr/local/google/home/natashenka/chromium/src/out/asan/../../base/message_loop/message_loop.cc:329:5
    #24 0x55bd4480a8d4 in base::MessageLoop::DoWork() /usr/local/google/home/natashenka/chromium/src/out/asan/../../base/message_loop/message_loop.cc:373:0
    #25 0x55bd44812bff in base::MessagePumpDefault::Run(base::MessagePump::Delegate*) /usr/local/google/home/natashenka/chromium/src/out/asan/../../base/message_loop/message_pump_default.cc:37:31
    #26 0x55bd4487fdb1 in base::RunLoop::Run() /usr/local/google/home/natashenka/chromium/src/out/asan/../../base/run_loop.cc:131:14
    #27 0x55bd448ff6c4 in base::Thread::ThreadMain() /usr/local/google/home/natashenka/chromium/src/out/asan/../../base/threading/thread.cc:337:3
    #28 0x55bd449ccaa4 in base::(anonymous namespace)::ThreadFunc(void*) /usr/local/google/home/natashenka/chromium/src/out/asan/../../base/threading/platform_thread_posix.cc:76:13
    #29 0x7fedd89fa493 in start_thread ??:0:0

1) unzip the attached webrtc-from-chat2.zip on a local webserver
2) fetch the webrtc source (https://webrtc.org/native-code/development/), and replace pc/srtptransport.cc and third_party/libsrtp/crypto/cipher/cipher.c with the version attached to the issue
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

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 H264, 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 H264, 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.

 
webrtcserver.py
3.7 KB View Download
webrtc-from-chat2.zip
397 KB Download
srtptransport.cc
18.3 KB View Download
cipher.c
22.2 KB View Download
Labels: Security_Severity-High M-67 Security_Impact-Stable Pri-1
Owner: tommi@chromium.org
Assigning from webrtc OWNERS file.
Cc: terelius@chromium.org tommi@chromium.org
Owner: philipel@chromium.org
Assigning to Philip who's likely the best person to take a look at this.

Terelius, if Philip has too much on his plate right now we may want to assign someone to help out with this.
I think recording RTP dumps would make things easier for both of us, so I created a patch (attached) you can use to dump incoming video RTP packets.

Before you apply the patch you have to modify line 50 and specify where you want the RTP dumps to be created.

Now whenever you run a test an RTP dump will be created. You can replay the dump by building 'ninja -C out/asan/ video_replay' and then run 'out/asan/video_replay --input_file /path/to/the/rtp/dump'

Now you only have to attach the RTP dump and we should be able to reproduce the problem :)




call_rtp_dump.diff
3.2 KB Download
Project Member

Comment 4 by sheriffbot@chromium.org, May 8 2018

Status: Assigned (was: Unconfirmed)
I tried this, and I couldn't get the issue to repro during replay. My suspicion is that there's something different about how the browser processes packets end-to-end and what the replay tool does. I've attached the packet dump anyhow in case it's useful. I'd also appreciate if you have any ideas on how to get replay to work end-to-end, this would be extremely useful in diagnosing these issues.
rtp_dump0_0x61d000672a80
376 KB View Download
It looks like the problem here is that this issue needs multiple decoders to occur, and the replay.cc only supports one. If there's a way to do this in replay.cc I'd appreciate if you let me know, otherwise I am working on a patch
Okay, I got this to work! The problem was indeed the config. I've written up a new replay.cc that takes an input as a parameter. You can reproduce this issue by calling:

./out/asan/video_replay --input_file  ~/dumps/rtp_dump0_0x61d0007b2a80  --config_file ~/config2.txt

with the attached files. Note that this needs to be built with the proprietary_codecs = true flag to work.

I think the ability to replay with the exact config is necessary to make the video_replay tool useful. Please let me know if you want me to commit this change to webrtc, I do think it would improve the ability to repro bugs a lot. 
rtp_dump0_0x61d0007b2a80
12.4 MB View Download
config2.txt
1.4 KB View Download
replay.cc
21.3 KB View Download
I got a crash in the code that parsed the config, so I updated my patch (quite) a bit.

With this patch both the RTP data and the stream config will be saved. The config is saved in a json format so we can easily parse it back.

To use the patch:
1. Edit line 61 to specify where you want to save RTP dumps/stream configs.
2. Apply the patch.
3. Run the test, now a /your/path/<some_name>_config and a /your/path/<some_name>_rtpdump will be created.
4. run video_replay --input_file /your/path/<some_name>

I converted the content of the config to json manually, but I didn't get a crash when I replayed the dump with those settings.

Hopefully, with this patch, we should be able to share reproducible test cases.
call_rtp_dump.diff
22.9 KB Download
Attached
test_rtpdump
12.4 MB View Download
test_config
2.8 KB View Download
Project Member

Comment 10 by bugdroid1@chromium.org, May 17 2018

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

commit 09133af36fba93691a22151765097c0bd581c1fa
Author: philipel <philipel@webrtc.org>
Date: Thu May 17 12:52:11 2018

Check number of nalus in packet before checking nalu types.

Bug:  chromium:840536 
Change-Id: Ia4dcf322ad6290691fd01b58fb02cd868714c92e
Reviewed-on: https://webrtc-review.googlesource.com/77121
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23283}
[modify] https://crrev.com/09133af36fba93691a22151765097c0bd581c1fa/modules/video_coding/packet_buffer.cc
[modify] https://crrev.com/09133af36fba93691a22151765097c0bd581c1fa/modules/video_coding/video_packet_buffer_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, May 17 2018

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

commit a37b9be2202fae823677bd09849dd7a3be506b7d
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Thu May 17 18:08:33 2018

Roll src/third_party/webrtc/ ef75ebef5..f8d8d6d00 (39 commits)

https://webrtc.googlesource.com/src.git/+log/ef75ebef5520..f8d8d6d00c16

$ git log ef75ebef5..f8d8d6d00 --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG= chromium:840536 ,chromium:755660,chromium:755660,chromium:755660,chromium:None,chromium:None,chromium:None,chromium:None,chromium:836790,chromium:None,chromium:None,chromium:None,chromium:None,chromium:None


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: Iec936924677ed8093fba2631fdef8a0041ad403c
Reviewed-on: https://chromium-review.googlesource.com/1064318
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@{#559607}
[modify] https://crrev.com/a37b9be2202fae823677bd09849dd7a3be506b7d/DEPS

Cc: huib@chromium.org
Cc: yfab...@apple.com
Philip, should this be closed?
Status: Fixed (was: Assigned)
Project Member

Comment 16 by sheriffbot@chromium.org, Jun 4

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

Comment 17 by sheriffbot@chromium.org, Jun 8

Labels: Merge-Request-68
Project Member

Comment 18 by sheriffbot@chromium.org, Jun 8

Labels: -Merge-Request-68 Hotlist-Merge-Review Merge-Review-68
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
I have no idea why the bot requested a merge to M68. AFAICT this fix is already in M68 (https://chromiumdash.appspot.com/commit/09133af36fba93691a22151765097c0bd581c1fa).
Please specify which OS's this is impacting, so merge request can be appropriately routed. 
Labels: -Merge-Review-68 Merge-Rejected-68
Per #20, this is already in branch. 
Labels: -M-67 M-68 Release-0-M68
Labels: OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
Cc: benwright@google.com
Labels: CVE-2018-6157 CVE_description-missing
Project Member

Comment 26 by sheriffbot@chromium.org, Sep 10

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