Bug 1197985 - Part 1 - Successfully skip ID3 tags stretching beyond the current input buffer. r=esawin

This also slightly tightens up invalid header detection for both ID3 and MPEG frame headers.

dom/media/MP3Demuxer.cpp

@@ -401,10 +401,18 @@ MP3TrackDemuxer::FindNextFrame() {
       // This is not a valid MPEG audio stream or we've reached EOS, give up.
       break;
     }
-    MOZ_ASSERT(mOffset + read > mOffset);
+    NS_ENSURE_TRUE(mOffset + read > mOffset, MediaByteRange(0, 0));
     mOffset += read;
     bufferEnd = buffer + read;
     frameBeg = mParser.Parse(buffer, bufferEnd);
+
+    if (frameBeg > bufferEnd) {
+      // We need to skip an ID3 tag which stretches beyond the current buffer.
+      const uint32_t bytesToSkip = frameBeg - bufferEnd;
+      NS_ENSURE_TRUE(mOffset + bytesToSkip > mOffset, MediaByteRange(0, 0));
+      mOffset += bytesToSkip;
+      frameBeg = bufferEnd;
+    }
   }
 
   if (frameBeg == bufferEnd || !mParser.CurrentFrame().Length()) {
@@ -618,7 +626,8 @@ FrameParser::Parse(const uint8_t* aBeg, const uint8_t* aEnd) {
     const uint8_t* id3Beg = mID3Parser.Parse(aBeg, aEnd);
     if (id3Beg != aEnd) {
       // ID3 headers found, skip past them.
-      aBeg = id3Beg + ID3Parser::ID3Header::SIZE + mID3Parser.Header().Size();
+      aBeg = id3Beg + ID3Parser::ID3Header::SIZE + mID3Parser.Header().Size() +
+             mID3Parser.Header().FooterSize();
     }
   }
 
@@ -633,9 +642,11 @@ FrameParser::Parse(const uint8_t* aBeg, const uint8_t* aEnd) {
     }
     // Move to the frame header begin to allow for whole-frame parsing.
     aBeg -= FrameHeader::SIZE;
-    return aBeg;
   }
-  return aEnd;
+  // If no headers (both ID3 and MP3) have been found, this is equivalent to returning aEnd.
+  // If we have found a large ID3 tag and want to skip past it, aBeg will point past the
+  // end of the buffer, which needs to be handled by the calling function.
+  return aBeg;
 }
 
 // FrameParser::Header
@@ -790,7 +801,7 @@ FrameParser::FrameHeader::ParseNext(uint8_t c) {
 
 bool
 FrameParser::FrameHeader::IsValid(int aPos) const {
-  if (IsValid()) {
+  if (aPos >= SIZE) {
     return true;
   }
   if (aPos == frame_header::SYNC1) {
@@ -802,7 +813,8 @@ FrameParser::FrameHeader::IsValid(int aPos) const {
            RawLayer() != 0;
   }
   if (aPos == frame_header::BITRATE_SAMPLERATE_PADDING_PRIVATE) {
-    return RawBitrate() != 0xF;
+    return RawBitrate() != 0xF && RawBitrate() != 0 &&
+           RawSampleRate() != 3;
   }
   return true;
 }
@@ -1017,6 +1029,14 @@ ID3Parser::ID3Header::Size() const {
   return mSize;
 }
 
+uint8_t
+ID3Parser::ID3Header::FooterSize() const {
+  if (Flags() & (1 << 4)) {
+    return SIZE;
+  }
+  return 0;
+}
+
 bool
 ID3Parser::ID3Header::ParseNext(uint8_t c) {
   if (!Update(c)) {
@@ -1030,7 +1050,7 @@ ID3Parser::ID3Header::ParseNext(uint8_t c) {
 
 bool
 ID3Parser::ID3Header::IsValid(int aPos) const {
-  if (IsValid()) {
+  if (aPos >= SIZE) {
     return true;
   }
   const uint8_t c = mRaw[aPos];

dom/media/MP3Demuxer.h

@@ -61,9 +61,12 @@ public:
     // The ID3 flags field.
     uint8_t Flags() const;
 
-    // The derived size based on the provides size fields.
+    // The derived size based on the provided size fields.
     uint32_t Size() const;
 
+    // Returns the size of an ID3v2.4 footer if present and zero otherwise.
+    uint8_t FooterSize() const;
+
     // Returns whether the parsed data is a valid ID3 header up to the given
     // byte position.
     bool IsValid(int aPos) const;
@@ -283,7 +286,9 @@ public:
   void EndFrameSession();
 
   // Parses given buffer [aBeg, aEnd) for a valid frame header.
-  // Returns begin of frame header if a frame header was found or aEnd otherwise.
+  // Returns begin of frame header if a frame header was found or a value >= aEnd otherwise.
+  // Values > aEnd indicate that additional bytes need to be skipped for jumping
+  // across an ID3 tag stretching beyond the given buffer.
   const uint8_t* Parse(const uint8_t* aBeg, const uint8_t* aEnd);
 
   // Parses given buffer [aBeg, aEnd) for a valid VBR header.