Dan Minor [:dminor] Reporter
	Description • 8 years ago

In Bug 1293422 I was inadvertently fuzzing Firefox with packets with missing headers, which caused heap corruption and an eventual crash. We should be rejecting invalid packets rather than crashing.

	Dan Minor [:dminor] Reporter
	Comment 1 • 8 years ago

jesup confirmed the same location reported in Bug 1293422:

    ==17052==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61900110258f at pc 0x00000049bc2a bp 0x7fffc433d550 sp 0x7fffc433cd10
    READ of size 12288 at 0x61900110258f thread T6 (Socket Thread)
    [New Thread 0x7fffc4346700 (LWP 17076)]
    Detaching after fork from child process 17223.
        #0 0x49bc29 in __asan_memcpy /home/nikola/final/llvm.src/projects/compiler-rt/lib/asan/asan_interceptors.cc:393:3
        #1 0x7fffd8bb687a in webrtc::VCMSessionInfo::Insert(unsigned char const*, unsigned long, bool, unsigned char*) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/video_coding/main/source/session_info.cc:222:3
        #2 0x7fffd8bb687a in webrtc::VCMSessionInfo::InsertBuffer(unsigned char*, std::_List_iterator<webrtc::VCMPacket>) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/video_coding/main/source/session_info.cc:193
        #3 0x7fffd8bb8868 in webrtc::VCMSessionInfo::InsertPacket(webrtc::VCMPacket const&, unsigned char*, webrtc::VCMDecodeErrorMode, webrtc::FrameData const&) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/video_coding/main/source/session_info.cc:565:25
        #4 0x7fffd8b8719d in webrtc::VCMFrameBuffer::InsertPacket(webrtc::VCMPacket const&, long, webrtc::VCMDecodeErrorMode, webrtc::FrameData const&) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/video_coding/main/source/frame_buffer.cc:144:18
        #5 0x7fffd8b93baf in webrtc::VCMJitterBuffer::InsertPacket(webrtc::VCMPacket const&, bool*) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/video_coding/main/source/jitter_buffer.cc:778:7
        #6 0x7fffd8bad1d2 in webrtc::VCMReceiver::InsertPacket(webrtc::VCMPacket const&, unsigned short, unsigned short) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/video_coding/main/source/receiver.cc:72:34
        #7 0x7fffd8bc0078 in webrtc::vcm::VideoReceiver::IncomingPacket(unsigned char const*, unsigned long, webrtc::WebRtcRTPHeader const&) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/video_coding/main/source/video_receiver.cc:580:17
        #8 0x7fffd8d2c634 in webrtc::ViEReceiver::OnReceivedPayloadData(unsigned char const*, unsigned long, webrtc::WebRtcRTPHeader const*) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/video_engine/vie_receiver.cc:249:7
        #9 0x7fffd8dad101 in webrtc::RTPReceiverVideo::ParseRtpPacket(webrtc::WebRtcRTPHeader*, webrtc::PayloadUnion const&, bool, unsigned char const*, unsigned long, long, bool) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc:101:10
        #10 0x7fffd8da9a9d in webrtc::RtpReceiverImpl::IncomingRtpPacket(webrtc::RTPHeader const&, unsigned char const*, unsigned long, webrtc::PayloadUnion, bool) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc:222:21
        #11 0x7fffd8d2cc74 in webrtc::ViEReceiver::ReceivePacket(unsigned char const*, unsigned long, webrtc::RTPHeader const&, bool) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/video_engine/vie_receiver.cc:358:10
        #12 0x7fffd8d2b9a4 in webrtc::ViEReceiver::InsertRTPPacket(unsigned char const*, unsigned long, webrtc::PacketTime const&) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/video_engine/vie_receiver.cc:332:13 


Submit a correction or amendment below (click here to make a fresh posting)
After submitting an amendment, you'll be able to view the differences between the old and new posts easily.

Syntax highlighting:


To highlight particular lines, prefix each line with ~~
==17052==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61900110258f at pc 0x00000049bc2a bp 0x7fffc433d550 sp 0x7fffc433cd10
READ of size 12288 at 0x61900110258f thread T6 (Socket Thread)
[New Thread 0x7fffc4346700 (LWP 17076)]
Detaching after fork from child process 17223.
    #0 0x49bc29 in __asan_memcpy /home/nikola/final/llvm.src/projects/compiler-rt/lib/asan/asan_interceptors.cc:393:3
    #1 0x7fffd8bb687a in webrtc::VCMSessionInfo::Insert(unsigned char const*, unsigned long, bool, unsigned char*) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/video_coding/main/source/session_info.cc:222:3
    #2 0x7fffd8bb687a in webrtc::VCMSessionInfo::InsertBuffer(unsigned char*, std::_List_iterator<webrtc::VCMPacket>) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/video_coding/main/source/session_info.cc:193
    #3 0x7fffd8bb8868 in webrtc::VCMSessionInfo::InsertPacket(webrtc::VCMPacket const&, unsigned char*, webrtc::VCMDecodeErrorMode, webrtc::FrameData const&) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/video_coding/main/source/session_info.cc:565:25
    #4 0x7fffd8b8719d in webrtc::VCMFrameBuffer::InsertPacket(webrtc::VCMPacket const&, long, webrtc::VCMDecodeErrorMode, webrtc::FrameData const&) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/video_coding/main/source/frame_buffer.cc:144:18
    #5 0x7fffd8b93baf in webrtc::VCMJitterBuffer::InsertPacket(webrtc::VCMPacket const&, bool*) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/video_coding/main/source/jitter_buffer.cc:778:7
    #6 0x7fffd8bad1d2 in webrtc::VCMReceiver::InsertPacket(webrtc::VCMPacket const&, unsigned short, unsigned short) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/video_coding/main/source/receiver.cc:72:34
    #7 0x7fffd8bc0078 in webrtc::vcm::VideoReceiver::IncomingPacket(unsigned char const*, unsigned long, webrtc::WebRtcRTPHeader const&) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/video_coding/main/source/video_receiver.cc:580:17
    #8 0x7fffd8d2c634 in webrtc::ViEReceiver::OnReceivedPayloadData(unsigned char const*, unsigned long, webrtc::WebRtcRTPHeader const*) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/video_engine/vie_receiver.cc:249:7
    #9 0x7fffd8dad101 in webrtc::RTPReceiverVideo::ParseRtpPacket(webrtc::WebRtcRTPHeader*, webrtc::PayloadUnion const&, bool, unsigned char const*, unsigned long, long, bool) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc:101:10
    #10 0x7fffd8da9a9d in webrtc::RtpReceiverImpl::IncomingRtpPacket(webrtc::RTPHeader const&, unsigned char const*, unsigned long, webrtc::PayloadUnion, bool) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc:222:21
    #11 0x7fffd8d2cc74 in webrtc::ViEReceiver::ReceivePacket(unsigned char const*, unsigned long, webrtc::RTPHeader const&, bool) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/video_engine/vie_receiver.cc:358:10
    #12 0x7fffd8d2b9a4 in webrtc::ViEReceiver::InsertRTPPacket(unsigned char const*, unsigned long, webrtc::PacketTime const&) /home/jesup/src/mozilla/asan-inbound/media/webrtc/trunk/webrtc/video_engine/vie_receiver.cc:332:13

	Randell Jesup [:jesup] (needinfo me) Assignee
	Comment 3 • 8 years ago

Comment on attachment 8780344 [details] [diff] [review]
Clean up H264 STAP-A handling

[ESR Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: Possible attack by feeding corrupt video data forcing an overrun with the data in the packets copied past the end of the buffer.

Fix Landed on Version: N/A

Risk to taking this patch (and alternatives if risky):  Low risk.  Adds some bufferspace, and kicks out (throws away data) if it's larger than it was supposed to be.

String or UUID changes made by this patch: none

[Feature/regressing bug #]: Upstream bug; N/A

[Describe test coverage new/current, TreeHerder]:  We don't fuzz in automation


[Security approval request comment]
How easily could an exploit be constructed based on the patch? Not that hard to figure out how to force a buffer-overrun.  Exploiting would likely require forcing a more complex case where there is a later packet in the "session" that needs to get moved down to make space for a corrupt STAP-A NAL.  However, normally a STAP-A is a single packet, so there'd be no later packets, but you might be able to trick it by sending a FU-A or other packet with a sequence number gap before it, then send the corrupt STAP-A as the packet to fill the sequence number gap (with the same timestamp as the packets after the gap).  This may not be obvious, but could be figured out with some work.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?  

The code and log messages do indicate buffersize issues and mishandling of corrupt STAP NAL sizes.

Which older supported branches are affected by this flaw? Pretty much all branches

If not all supported branches, which bug introduced the flaw?  Probably the upstream 43 import, or perhaps the one before that.  At least back to Nov 2015.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?  Backports should be trivial; few changes to these files.

How likely is this patch to cause regressions; how much testing does it need?  Some manual testing - I've 'fuzzed' it by using the broken Mode 0 support that generates corrupt packets, and I could previously see ASAN hits quickly (seconds or instantly).  With this patch it's gone for a good part of an hour with no problems.  Also, I've run it with Mode 1 to verify it's good under ASAN, no problems.


A more complete fuzzing of H264 would be a good idea as a followup.

	Randell Jesup [:jesup] (needinfo me) Assignee
	Comment 4 • 8 years ago

 https://hg.mozilla.org/integration/mozilla-inbound/rev/f77d344cf887

	Randell Jesup [:jesup] (needinfo me) Assignee
	Comment 5 • 8 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/d23f68f5cb77
https://hg.mozilla.org/releases/mozilla-beta/rev/8ea0c38aae0b

	Ryan VanderMeulen [:RyanVM]
	Comment 6 • 8 years ago

https://hg.mozilla.org/releases/mozilla-esr45/rev/dd9eb81853b9

	Ryan VanderMeulen [:RyanVM]
	Comment 7 • 8 years ago

https://hg.mozilla.org/mozilla-central/rev/f77d344cf887

	Dan Minor [:dminor] Reporter
	Comment 8 • 6 years ago

We should upstream this fix.