Bug 1220493 - validate RTP packets against underflows. r=pkerr a=lizzard
authorRandell Jesup <rjesup@jesup.org>
Thu, 05 Nov 2015 10:17:29 -0500
changeset 305584 575d3aa376b1c8e7507d94833f7b74bf963127cb
parent 305583 8f0b0f852ca1e6cde52ae6ecb62cf9b02d094fe3
child 305585 1d414d707af3828c6bb37c1833f374c1a6048ef3
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspkerr, lizzard
bugs1220493
milestone44.0a2
Bug 1220493 - validate RTP packets against underflows. r=pkerr a=lizzard
media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_utility.cc
--- a/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_utility.cc
+++ b/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_utility.cc
@@ -333,39 +333,40 @@ bool RtpHeaderParser::Parse(RTPHeader& h
   SSRC += *ptr++ << 16;
   SSRC += *ptr++ << 8;
   SSRC += *ptr++;
 
   if (V != kRtpExpectedVersion) {
     return false;
   }
 
-  const uint8_t CSRCocts = CC * 4;
-
-  if ((ptr + CSRCocts) > _ptrRTPDataEnd) {
-    return false;
-  }
-
   header.markerBit      = M;
   header.payloadType    = PT;
   header.sequenceNumber = sequenceNumber;
   header.timestamp      = RTPTimestamp;
   header.ssrc           = SSRC;
   header.numCSRCs       = CC;
   header.paddingLength  = P ? *(_ptrRTPDataEnd - 1) : 0;
 
+  // 12 == sizeof(RFC rtp header) == kRtpMinParseLength, each CSRC=4 bytes
+  header.headerLength   = 12 + (CC * 4);
+  // not a full validation, just safety against underflow.  Padding must
+  // start after the header.  We can have 0 payload bytes left, note.
+  if (header.paddingLength + header.headerLength > length) {
+    return false;
+  }
+
   for (unsigned int i = 0; i < CC; ++i) {
     uint32_t CSRC = *ptr++ << 24;
     CSRC += *ptr++ << 16;
     CSRC += *ptr++ << 8;
     CSRC += *ptr++;
     header.arrOfCSRCs[i] = CSRC;
   }
-
-  header.headerLength   = 12 + CSRCocts;
+  assert((ptr - _ptrRTPDataBegin) == header.headerLength);
 
   // If in effect, MAY be omitted for those packets for which the offset
   // is zero.
   header.extension.hasTransmissionTimeOffset = false;
   header.extension.transmissionTimeOffset = 0;
 
   // May not be present in packet.
   header.extension.hasAbsoluteSendTime = false;
@@ -380,31 +381,32 @@ bool RtpHeaderParser::Parse(RTPHeader& h
      0                   1                   2                   3
      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |      defined by profile       |           length              |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |                        header extension                       |
     |                             ....                              |
     */
-    const ptrdiff_t remain = _ptrRTPDataEnd - ptr;
-    if (remain < 4) {
+    // earlier test ensures we have at least paddingLength bytes left
+    const ptrdiff_t remain = (_ptrRTPDataEnd - ptr) - header.paddingLength;
+    if (remain < 4) { // minimum header extension length = 32 bits
       return false;
     }
 
     header.headerLength += 4;
 
     uint16_t definedByProfile = *ptr++ << 8;
     definedByProfile += *ptr++;
 
-    uint16_t XLen = *ptr++ << 8;
+    size_t XLen = *ptr++ << 8;
     XLen += *ptr++; // in 32 bit words
     XLen *= 4; // in octs
 
-    if (remain < (4 + XLen)) {
+    if (remain < (4 + XLen)) { // we already accounted for padding
       return false;
     }
     if (definedByProfile == kRtpOneByteHeaderExtensionId) {
       const uint8_t* ptrRTPDataExtensionEnd = ptr + XLen;
       ParseOneByteExtensionHeader(header,
                                   ptrExtensionMap,
                                   ptrRTPDataExtensionEnd,
                                   ptr);