Security: WebRTC: Unchecked Optional Access in Updating timestamp after RED packet |
|||||||||
Issue descriptionThe attached file causes an unchecked access to an Optional object when processing rtp packets. To reproduce the issue, run: neteq_rtpplay --g722 9 --pcmu 0 --pcma 8 --isac 103 --isac 104 --avt 110 --opus 111 --avt 112 --avt 113 --cn_nb 105 --avt 126 unknown soob.pcm on the attached file. This issue is probably not exploitable, but I'm filing it under the security component just in case.
,
Jun 12 2018
Hmm I'm not able to repro this. I get this output: $ out/asan64/neteq_rtpplay --g722 9 --pcmu 0 --pcma 8 --isac 103 --isac 104 --avt 110 --opus 111 --avt 112 --avt 113 --cn_nb 105 --avt 126 unknown soob.pcm Input file: unknown Found valid packet with payload type 117 and SSRC 0x312e3020 Output file: soob.pcm [0612/133923.708213:ERROR:neteq_impl.cc(117)] Sample rate 0 Hz not supported. Changing to 8000 Hz. # # Fatal error in ../../third_party/webrtc/modules/audio_coding/neteq/tools/neteq_test.cc, line 122 # last system error: 0 # Check failed: neteq_->RegisterPayloadType(c.second.first, c.second.second, c.first) == NetEq::kOK (-1 vs. 0) # Cannot register ilbc to payload type 102 # Aborted -- I had to patch my local webrtc (under chromium) checkout to get the neteq_rtpplay target compiling. The patches I used are here: https://webrtc-review.googlesource.com/#/c/src/+/83260 I am on rev 9dce71b9 of webrtc.
,
Jun 12 2018
ah, this is because chromium overrides webrtc and disables ilbc: https://cs.chromium.org/chromium/src/.gn?type=cs&l=65 now I get a crash, but it doesn't symbolize yet.
,
Jun 12 2018
This is a NULL Ptr deref at: #0 0x00000000005c8f35 in InsertPacketInternal() () at ../../third_party/webrtc/modules/audio_coding/neteq/neteq_impl.cc:681 #1 0x00000000005c7d68 in InsertPacket() () at ../../third_party/webrtc/modules/audio_coding/neteq/neteq_impl.cc:145 #2 0x00000000005121af in Run() () at ../../third_party/webrtc/modules/audio_coding/neteq/tools/neteq_test.cc:61 #3 0x0000000000488328 in RunTest() () at ../../third_party/webrtc/modules/audio_coding/neteq/tools/neteq_rtpplay.cc:474 #4 0x00007ffff59e02b1 in __libc_start_main () at /lib/x86_64-linux-gnu/libc.so.6 #5 0x00000000003ac02a in _start () looks like a NULL deref here: https://cs.chromium.org/chromium/src/third_party/webrtc/modules/audio_coding/neteq/neteq_impl.cc?l=681 I agree I don't think this is exploitable - is this crash the same one you are getting, natashenka?
,
Jun 12 2018
No, there's an OOB asan crash ... are you testing with ASAN?
,
Jun 12 2018
yes this is an asan build but it's a chromium checkout, perhaps I need to try with a webrtc checkout.
,
Jun 12 2018
I used a WebRTC checkout to test this, that's probably the easiest.
,
Jun 14 2018
,
Jun 14 2018
I tried with a webrtc checkout and still get the same failure at the same place. Do I need a real hardware device (I don't have that).
,
Jun 14 2018
perhaps it would be easier if you just posted the asan stack?
,
Jun 14 2018
Needs a reliable repro case to proceed here. Assigning to guidou@ to follow up with the reporter. Based on the comment in #0 I'm triaging as Low, but once we see the asan stack, we can reassess if needed.
,
Jun 14 2018
This reproduces on a WebRTC checkout built on Linux for me. I'm OOO, I'll post the ASAN stack when I'm back on Tuesday.
,
Jun 15 2018
Will wait for the stack trace before reassigning to someone from the audio team.
,
Jun 15 2018
,
Jun 19 2018
The asan dump I'm seeing is:
==81313==ERROR: AddressSanitizer: unknown-crash on address 0x6100000011ec at pc 0x00000062000f bp 0x7fff89f2ec50 sp 0x7fff89f2ec48
READ of size 4 at 0x6100000011ec thread T0
#0 0x62000e in SampleRateHz modules/audio_coding/neteq/decoder_database.h:79:64
#1 0x62000e in webrtc::TimestampScaler::ToInternal(unsigned int, unsigned char) modules/audio_coding/neteq/timestamp_scaler.cc:46
#2 0x61fa74 in webrtc::TimestampScaler::ToInternal(webrtc::Packet*) modules/audio_coding/neteq/timestamp_scaler.cc:26:23
#3 0x61fb7c in webrtc::TimestampScaler::ToInternal(std::__1::list<webrtc::Packet, std::__1::allocator<webrtc::Packet> >*) modules/audio_coding/neteq/timestamp_scaler.cc:32:5
#4 0x5c8c72 in webrtc::NetEqImpl::InsertPacketInternal(webrtc::RTPHeader const&, rtc::ArrayView<unsigned char const, -4711l>, unsigned int) modules/audio_coding/neteq/neteq_impl.cc:645:24
#5 0x5c7e8d in webrtc::NetEqImpl::InsertPacket(webrtc::RTPHeader const&, rtc::ArrayView<unsigned char const, -4711l>, unsigned int) modules/audio_coding/neteq/neteq_impl.cc:145:7
#6 0x4f146e in webrtc::test::NetEqTest::Run() modules/audio_coding/neteq/tools/neteq_test.cc:61:27
#7 0x452668 in webrtc::test::(anonymous namespace)::RunTest(int, char**) modules/audio_coding/neteq/tools/neteq_rtpplay.cc:474:35
#8 0x7f369c7c12b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
0x6100000011ec is located 172 bytes inside of 184-byte region [0x610000001140,0x6100000011f8)
allocated by thread T0 here:
#0 0x447f72 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 0x5f6986 in __libcpp_allocate buildtools/third_party/libc++/trunk/include/new:259:10
#2 0x5f6986 in allocate buildtools/third_party/libc++/trunk/include/memory:1799
#3 0x5f6986 in allocate buildtools/third_party/libc++/trunk/include/memory:1548
#4 0x5f6986 in __construct_node<std::__1::pair<unsigned char, webrtc::DecoderDatabase::DecoderInfo> > buildtools/third_party/libc++/trunk/include/__tree:2191
#5 0x5f6986 in std::__1::pair<std::__1::__tree_iterator<std::__1::__value_type<unsigned char, webrtc::DecoderDatabase::DecoderInfo>, std::__1::__tree_node<std::__1::__value_type<unsigned char, webrtc::DecoderDatabase::DecoderInfo>, void*>*, long>, bool> std::__1::__tree<std::__1::__value_type<unsigned char, webrtc::DecoderDatabase::DecoderInfo>, std::__1::__map_value_compare<unsigned char, std::__1::__value_type<unsigned char, webrtc::DecoderDatabase::DecoderInfo>, std::__1::less<unsigned char>, true>, std::__1::allocator<std::__1::__value_type<unsigned char, webrtc::DecoderDatabase::DecoderInfo> > >::__emplace_unique_key_args<unsigned char, std::__1::pair<unsigned char, webrtc::DecoderDatabase::DecoderInfo> >(unsigned char const&, std::__1::pair<unsigned char, webrtc::DecoderDatabase::DecoderInfo>&&) buildtools/third_party/libc++/trunk/include/__tree:2137
#6 0x5f361f in __emplace_unique_extract_key<std::__1::pair<unsigned char, webrtc::DecoderDatabase::DecoderInfo> > buildtools/third_party/libc++/trunk/include/__tree:1214:14
#7 0x5f361f in __emplace_unique<std::__1::pair<unsigned char, webrtc::DecoderDatabase::DecoderInfo> > buildtools/third_party/libc++/trunk/include/__tree:1176
#8 0x5f361f in __insert_unique<std::__1::pair<unsigned char, webrtc::DecoderDatabase::DecoderInfo>, void> buildtools/third_party/libc++/trunk/include/__tree:1304
#9 0x5f361f in insert<std::__1::pair<unsigned char, webrtc::DecoderDatabase::DecoderInfo>, void> buildtools/third_party/libc++/trunk/include/map:1054
#10 0x5f361f in webrtc::DecoderDatabase::RegisterPayload(unsigned char, webrtc::NetEqDecoder, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) modules/audio_coding/neteq/decoder_database.cc:198
#11 0x5cfdcd in webrtc::NetEqImpl::RegisterPayloadType(webrtc::NetEqDecoder, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned char) modules/audio_coding/neteq/neteq_impl.cc:240:26
#12 0x4f02c7 in webrtc::test::NetEqTest::RegisterDecoders(std::__1::map<int, std::__1::pair<webrtc::NetEqDecoder, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::less<int>, std::__1::allocator<std::__1::pair<int const, std::__1::pair<webrtc::NetEqDecoder, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > > > const&) modules/audio_coding/neteq/tools/neteq_test.cc:120:5
#13 0x4efe8a in webrtc::test::NetEqTest::NetEqTest(webrtc::NetEq::Config const&, std::__1::map<int, std::__1::pair<webrtc::NetEqDecoder, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::less<int>, std::__1::allocator<std::__1::pair<int const, std::__1::pair<webrtc::NetEqDecoder, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > > > const&, std::__1::map<int, webrtc::test::NetEqTest::ExternalDecoderInfo, std::__1::less<int>, std::__1::allocator<std::__1::pair<int const, webrtc::test::NetEqTest::ExternalDecoderInfo> > > const&, std::__1::unique_ptr<webrtc::test::NetEqInput, std::__1::default_delete<webrtc::test::NetEqInput> >, std::__1::unique_ptr<webrtc::test::AudioSink, std::__1::default_delete<webrtc::test::AudioSink> >, webrtc::test::NetEqTest::Callbacks) modules/audio_coding/neteq/tools/neteq_test.cc:45:3
#14 0x452576 in webrtc::test::(anonymous namespace)::RunTest(int, char**) modules/audio_coding/neteq/tools/neteq_rtpplay.cc:471:13
#15 0x7f369c7c12b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
SUMMARY: AddressSanitizer: unknown-crash modules/audio_coding/neteq/decoder_database.h:79:64 in SampleRateHz
Shadow bytes around the buggy address:
0x0c207fff81e0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
0x0c207fff81f0: 00 00 00 00 00 00 00 00 00 f7 00 00 00 04 00 fa
0x0c207fff8200: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
0x0c207fff8210: 00 00 00 00 00 00 00 00 00 f7 00 00 00 04 00 fa
0x0c207fff8220: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
=>0x0c207fff8230: 00 00 00 00 00 00 00 00 00 f7 00 00 00[04]00 fa
0x0c207fff8240: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
0x0c207fff8250: 00 00 00 00 00 00 00 00 00 f7 00 00 00 04 00 fa
0x0c207fff8260: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c207fff8270: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c207fff8280: 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
Shadow gap: cc
==81313==ABORTING
The root cause here is that in modules/audio_coding/neteq/decoder_database.h there's the following code:
const AudioDecoder* decoder = GetDecoder();
RTC_DCHECK_EQ(1, !!decoder + !!cng_decoder_);
return decoder ? decoder->SampleRateHz() : cng_decoder_->sample_rate_hz
}
If both decoder and cng_decoder_ are not defined, the DCHECK fires, but in release, the next line executes, and since decoder is NULL,
cng_decoder_->sample_rate_hz
is executed. cng_decoder_ is an empty optional, so this reads out of bounds.
,
Jun 20 2018
natashenka@: Thanks a lot for the trace. hlundin@: Please take a look or triage further.
,
Jul 3
The problem with the input file is that the "Block PT" (see https://tools.ietf.org/html/rfc2198.html#section-3) is 117, which maps to RED. That is, the RED packet's first subpart is itself tagged as a RED packet. NetEq crashes on this, which it should of course not. I will fix that. For reference, this issue can be reproduced without ASAN. Simply set dcheck_always_on = true in GN arguments, compile neteq_rtpplay and run the command in the OP.
,
Jul 3
,
Jul 3
The following revision refers to this bug: https://webrtc.googlesource.com/src.git/+/defa7a80491c8598d74ba2895bdb73d7d1e674f3 commit defa7a80491c8598d74ba2895bdb73d7d1e674f3 Author: Henrik Lundin <henrik.lundin@webrtc.org> Date: Tue Jul 03 20:27:57 2018 NetEq: Handle nested RED packets This CL makes NetEq handle nested RED packets without crashing. Nested RED packets mean that the block PT (see https://tools.ietf.org/html/rfc2198.html#section-3) in the RED packet is also set to the RED PT. This implies a nested RED packet, which is not supported. Instead, all payloads in a RED packet that have the RED PT will be discarded. Bug: chromium:851662 Change-Id: I86ec257e60fb8076e3574ac5a4a1ca50196f6b34 Reviewed-on: https://webrtc-review.googlesource.com/86824 Commit-Queue: Henrik Lundin <henrik.lundin@webrtc.org> Reviewed-by: Ivo Creusen <ivoc@webrtc.org> Cr-Commit-Position: refs/heads/master@{#23825} [modify] https://crrev.com/defa7a80491c8598d74ba2895bdb73d7d1e674f3/modules/audio_coding/neteq/mock/mock_red_payload_splitter.h [modify] https://crrev.com/defa7a80491c8598d74ba2895bdb73d7d1e674f3/modules/audio_coding/neteq/neteq_impl.cc [modify] https://crrev.com/defa7a80491c8598d74ba2895bdb73d7d1e674f3/modules/audio_coding/neteq/red_payload_splitter.cc [modify] https://crrev.com/defa7a80491c8598d74ba2895bdb73d7d1e674f3/modules/audio_coding/neteq/red_payload_splitter.h [modify] https://crrev.com/defa7a80491c8598d74ba2895bdb73d7d1e674f3/modules/audio_coding/neteq/red_payload_splitter_unittest.cc
,
Jul 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/47f7073d7af4edcefd0c554d514bc5e50f83726e commit 47f7073d7af4edcefd0c554d514bc5e50f83726e Author: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Date: Wed Jul 04 01:24:04 2018 Roll src/third_party/webrtc d000b0a32ee7..57900cb93360 (6 commits) https://webrtc.googlesource.com/src.git/+log/d000b0a32ee7..57900cb93360 git log d000b0a32ee7..57900cb93360 --date=short --no-merges --format='%ad %ae %s' 2018-07-03 buildbot@webrtc.org Roll chromium_revision 6df2efa531..79cbcdf6fb (572277:572378) 2018-07-03 shampson@webrtc.org Adding shampson (me) as an owner to pc/ & api/. 2018-07-03 henrik.lundin@webrtc.org NetEq: Handle nested RED packets 2018-07-03 shampson@webrtc.org Adding ICE configurations to the PC perf test. 2018-07-03 buildbot@webrtc.org Roll chromium_revision ce19c6d80b..6df2efa531 (572160:572277) 2018-07-03 srte@webrtc.org Adds debug printing for congestion controllers. Created with: gclient setdep -r src/third_party/webrtc@57900cb93360 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=luci.chromium.try:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng BUG=chromium:None,chromium:None,chromium:851662,chromium:None TBR=webrtc-chromium-sheriffs-robots@google.com Change-Id: Id33803de30fd9ae7b5196b0337c6740324a3a3ad Reviewed-on: https://chromium-review.googlesource.com/1125419 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@{#572430} [modify] https://crrev.com/47f7073d7af4edcefd0c554d514bc5e50f83726e/DEPS
,
Jul 4
natashenka@, thanks for reporting! I believe I have fixed the issue with the above CL. If you can verify, I would appreciate your help.
,
Jul 4
,
Oct 11
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 palmer@chromium.org
, Jun 11 2018Labels: OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows