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

Issue 841962 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: WebRTC: Overflow in FEC Processing

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

Issue description

There are several calls to memcpy that can overflow the destination buffer in webrtc::UlpfecReceiverImpl::AddReceivedRedPacket. The method takes a parameter incoming_rtp_packet, which is an RTP packet with a mac length that is defined by the transport (2048 bytes for DTLS in Chrome). This packet is then copied to the received_packet in several locations in the method, depending on packet properties, using the lenth of the incoming_rtp_packet as the copy length. The received_packet is a ForwardErrorCorrection::ReceivedPacket, which has a max size of 1500. Therefore, the memcpy calls in this method can overflow this buffer.

==204614==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61b000046670 at pc 0x00000059d958 bp 0x7ffcac5716f0 sp 0x7ffcac570ea0
WRITE of size 2316 at 0x61b000046670 thread T0
    #0 0x59d957 in __asan_memcpy /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cc:23:3
    #1 0x1b6aacc in webrtc::UlpfecReceiverImpl::AddReceivedRedPacket(webrtc::RTPHeader const&, unsigned char const*, unsigned long, unsigned char) modules/rtp_rtcp/source/ulpfec_receiver_impl.cc:173:5
    #2 0x1b3cd5c in webrtc::RtpVideoStreamReceiver::ParseAndHandleEncapsulatingHeader(unsigned char const*, unsigned long, webrtc::RTPHeader const&) video/rtp_video_stream_receiver.cc:426:27
    #3 0x1b39a31 in webrtc::RtpVideoStreamReceiver::ReceivePacket(unsigned char const*, unsigned long, webrtc::RTPHeader const&) video/rtp_video_stream_receiver.cc:402:5
    #4 0x1b3a895 in webrtc::RtpVideoStreamReceiver::OnRtpPacket(webrtc::RtpPacketReceived const&) video/rtp_video_stream_receiver.cc:301:3
    #5 0x8c7a26 in webrtc::RtpDemuxer::OnRtpPacket(webrtc::RtpPacketReceived const&) call/rtp_demuxer.cc:157:11
    #6 0x8cec3d in webrtc::RtpStreamReceiverController::OnRtpPacket(webrtc::RtpPacketReceived const&) call/rtp_stream_receiver_controller.cc:55:19
    #7 0x12e8507 in webrtc::internal::Call::DeliverRtp(webrtc::MediaType, rtc::CopyOnWriteBuffer, webrtc::PacketTime const&) call/call.cc:1291:36
    #8 0x12e92a0 in webrtc::internal::Call::DeliverPacket(webrtc::MediaType, rtc::CopyOnWriteBuffer, webrtc::PacketTime const&) call/call.cc:1316:10
    #9 0x5da2a6 in webrtc::RtpReplay() video/replay.cc:635:31
    #10 0x5dd5fe in main video/replay.cc:700:3
    #11 0x7feaa1ee92b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

0x61b000046670 is located 0 bytes to the right of 1520-byte region [0x61b000046080,0x61b000046670)
allocated by thread T0 here:
    #0 0x5c9362 in operator new(unsigned long) /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/asan/asan_new_delete.cc:93:3
    #1 0x1b6a8c8 in webrtc::UlpfecReceiverImpl::AddReceivedRedPacket(webrtc::RTPHeader const&, unsigned char const*, unsigned long, unsigned char) modules/rtp_rtcp/source/ulpfec_receiver_impl.cc:165:35
    #2 0x1b3cd5c in webrtc::RtpVideoStreamReceiver::ParseAndHandleEncapsulatingHeader(unsigned char const*, unsigned long, webrtc::RTPHeader const&) video/rtp_video_stream_receiver.cc:426:27
    #3 0x1b39a31 in webrtc::RtpVideoStreamReceiver::ReceivePacket(unsigned char const*, unsigned long, webrtc::RTPHeader const&) video/rtp_video_stream_receiver.cc:402:5
    #4 0x1b3a895 in webrtc::RtpVideoStreamReceiver::OnRtpPacket(webrtc::RtpPacketReceived const&) video/rtp_video_stream_receiver.cc:301:3
    #5 0x8c7a26 in webrtc::RtpDemuxer::OnRtpPacket(webrtc::RtpPacketReceived const&) call/rtp_demuxer.cc:157:11
    #6 0x8cec3d in webrtc::RtpStreamReceiverController::OnRtpPacket(webrtc::RtpPacketReceived const&) call/rtp_stream_receiver_controller.cc:55:19
    #7 0x12e8507 in webrtc::internal::Call::DeliverRtp(webrtc::MediaType, rtc::CopyOnWriteBuffer, webrtc::PacketTime const&) call/call.cc:1291:36
    #8 0x12e92a0 in webrtc::internal::Call::DeliverPacket(webrtc::MediaType, rtc::CopyOnWriteBuffer, webrtc::PacketTime const&) call/call.cc:1316:10
    #9 0x5da2a6 in webrtc::RtpReplay() video/replay.cc:635:31
    #10 0x5dd5fe in main video/replay.cc:700:3
    #11 0x7feaa1ee92b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

SUMMARY: AddressSanitizer: heap-buffer-overflow /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cc:23:3 in __asan_memcpy
Shadow bytes around the buggy address:
  0x0c3680000c70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3680000c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3680000c90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3680000ca0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3680000cb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c3680000cc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00[fa]fa
  0x0c3680000cd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3680000ce0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3680000cf0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3680000d00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3680000d10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb


To reproduce this issue:

1) replace video/replay.cc with the attached version, and build it with asan (ninja -C out/asan video_replay). Note that this file adds the ability to load a full receiver config to the video replay tool, I'm hoping to eventually get this change committed to WebRTC.

2) Download the attached files config4.txt and fallbackoob1

3) run video_replay --input_file  fallbackoob1  --config_file config4.txt

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.
 
fallbackoob1
13.4 KB View Download
replay.cc
21.3 KB View Download
config4.txt
1.5 KB View Download
Components: Blink>WebRTC>Video

Comment 2 by sprang@chromium.org, May 11 2018

Owner: brandtr@chromium.org
Status: Assigned (was: Unconfirmed)
brandtr@ you are familiar with the FEC stack, can you triage?

Comment 3 by holmer@chromium.org, May 11 2018

natashenka, how do you reproduce this in Chrome? It surprises me that we'd ever get packets larger than 1500 bytes in the RTP layer, so it sounds to me like this should be fixed somewhere else as well. I'd guess the RTP stack assumes no larger packets than 1500 bytes in many places.
I agree, restricting packet size overall would be an ideal solution. To reproduce this issue in Chrome:

1) sync a new copy of webrtc
2) edit pc/srtptransport.cc to add the following code on line 123, inside SendRtpPacket (sample attached) and build

  size_t temp = packet->size();
  packet->EnsureCapacity(2048);
  packet->SetSize(2034);
  uint8_t* data = packet->data();
  for(size_t t = temp; t < 2034; t++){
    data[t] = 0x77;
  }

3) Download and run webrtcserver.py
4) Unzip webrtc-from-chat.zip on a webserver
5) run peerconnection_client from your webrtc build above, and connect to the local server, and select test2
6) visit http://127.0.0.1/webrtc-from-chat/index.html from the target browser
7) enter any username and select "Log in", and then enter text into the chat bar and press send
8) Boom!
srtptransport.cc
18.3 KB View Download
webrtcserver.py
3.8 KB View Download
webrtc-from-chat.zip
397 KB Download

Comment 5 by rsesek@chromium.org, May 13 2018

Labels: M-67 Security_Severity-High OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows Pri-1

Comment 6 by rsesek@chromium.org, May 13 2018

Labels: Security_Impact-Stable
natashenka: I can repro with video_replay, but when trying with webrtcserver.py I get the following when trying to join from the browser:
Error in connection handler
Traceback (most recent call last):
  File "/usr/local/google/home/brandtr/.virtualenvs/webrtc/lib/python3.5/site-packages/websockets/server.py", line 135, in handler
    yield from self.ws_handler(self, path)
  File "webrtcserver.py", line 77, in echo
    async for message in websocket:
TypeError: 'async for' requires an object with __aiter__ method, got WebSocketServerProtocol

What websockets version are you using? I tried with 4.0.1.
Cc: brandtr@chromium.org holmer@chromium.org
Owner: yinwa@chromium.org
Ying: can you take a look at fixing this in UlpfecReceiverImpl?
Cc: crodbro@chromium.org
Sorry, forgot to mention, you need to use python3.6 or higher
Alright, that probably explains it! Seems that rodete only ships with 3.5.3.
Yeah, 3.6 was fairly easy to install from source though

Comment 13 by yinwa@chromium.org, May 18 2018

I am looking into this.
Project Member

Comment 14 by bugdroid1@chromium.org, May 22 2018

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

commit 7a84fcf47a492d17ca20947e65b21a06b28e77cd
Author: Ying Wang <yinwa@webrtc.org>
Date: Tue May 22 09:32:18 2018

Prevent potential buffer overflow in UlpfecReceiver

Bug:  chromium:841962 
Change-Id: I5ef0341a5fffe6b6204f5b2edbaec2d389a56964
Reviewed-on: https://webrtc-review.googlesource.com/77420
Commit-Queue: Ying Wang <yinwa@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23341}
[modify] https://crrev.com/7a84fcf47a492d17ca20947e65b21a06b28e77cd/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc

Comment 15 by yinwa@chromium.org, May 22 2018

Status: Fixed (was: Assigned)
hi natashenka, 

I tested with your video_replay. This shall fix the heap overflow issue. However the video_replay will break at rtp_file_reader.cc line 181. As the packet length in the test input file exceed the maximum length 3500.
Project Member

Comment 16 by sheriffbot@chromium.org, May 22 2018

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Thanks! That's fine, it doesn't need to work, just not corrupt memory
Project Member

Comment 18 by bugdroid1@chromium.org, May 23 2018

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

commit 349158ec7d7da1de29628c682e1b26a476e6381d
Author: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed May 23 15:38:33 2018

Roll src/third_party/webrtc/ a832019f4..547e3169d (45 commits)

https://webrtc.googlesource.com/src.git/+log/a832019f4e3a..547e3169d9e0

$ git log a832019f4..547e3169d --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG= chromium:813328 , chromium:845135 ,chromium:none,chromium:841962,chromium:888042,chromium:None,chromium:845158,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: I1d118a77cf2b61247d8720d3c9b93091416454a9
Reviewed-on: https://chromium-review.googlesource.com/1070209
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@{#561102}
[modify] https://crrev.com/349158ec7d7da1de29628c682e1b26a476e6381d/DEPS

Labels: -M-67 M-68
Project Member

Comment 20 by sheriffbot@chromium.org, Jun 8

Labels: Merge-Request-68
Project Member

Comment 21 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
Labels: -Merge-Review-68 Merge-Approved-68
Approved - branch:3440
Project Member

Comment 23 by sheriffbot@chromium.org, Jun 12

Cc: abdulsyed@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 24 by sheriffbot@chromium.org, Jun 18

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: abdulsyed@chromium.org
Pls merge you change to M68 branch 3440 ASAP so we can pick it up for this week Beta release. Merge has to happen latest by 1:00 PM PT tomorrow, Tuesday (06/19), so we can pick it up for Wednesday Beta release.




It looks like is already in M68:
https://webrtc.googlesource.com/src/+log/branch-heads/68, search for: 7a84fcf
Labels: -Merge-Approved-68
Labels: Release-0-M68
Labels: CVE-2018-6156 CVE_description-missing
Project Member

Comment 30 by sheriffbot@chromium.org, Aug 28

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