Set error on unrecognized top-level BMFF boxes.

A common error when using BMFF with Media Source is to begin appending at an
incorrect offset, such that the parser sees an extremely large value for the
box size and consequently waits forever for input without failing. This change
adds a whitelist of permitted top-level boxes, which allows these situations
to be detected as soon as the header is read.

BUG=
TEST=BoxReaderTest.WrongFourCCTest


Review URL: https://chromiumcodereview.appspot.com/10823139

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149735 0039d316-1c4b-4281-b951-d872f2087c98

media/mp4/box_reader.cc

@@ -10,6 +10,7 @@
 #include <set>
 
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
 #include "media/mp4/box_definitions.h"
 #include "media/mp4/rcheck.h"
 
@@ -95,14 +96,22 @@ BoxReader::~BoxReader() {
   }
 }
 
+// static
 BoxReader* BoxReader::ReadTopLevelBox(const uint8* buf,
                                       const int buf_size,
                                       bool* err) {
-  BoxReader* reader = new BoxReader(buf, buf_size);
-  if (reader->ReadHeader(err) && reader->size() <= buf_size) {
-    return reader;
+  scoped_ptr<BoxReader> reader(new BoxReader(buf, buf_size));
+  if (!reader->ReadHeader(err))
+    return NULL;
+
+  if (!IsValidTopLevelBox(reader->type())) {
+    *err = true;
+    return NULL;
   }
-  delete reader;
+
+  if (reader->size() <= buf_size)
+    return reader.release();
+
   return NULL;
 }
 
@@ -114,11 +123,41 @@ bool BoxReader::StartTopLevelBox(const uint8* buf,
                                  bool* err) {
   BoxReader reader(buf, buf_size);
   if (!reader.ReadHeader(err)) return false;
+  if (!IsValidTopLevelBox(reader.type())) {
+    *err = true;
+    return false;
+  }
   *type = reader.type();
   *box_size = reader.size();
   return true;
 }
 
+// static
+bool BoxReader::IsValidTopLevelBox(const FourCC& type) {
+  switch (type) {
+    case FOURCC_FTYP:
+    case FOURCC_PDIN:
+    case FOURCC_MOOV:
+    case FOURCC_MOOF:
+    case FOURCC_MFRA:
+    case FOURCC_MDAT:
+    case FOURCC_FREE:
+    case FOURCC_SKIP:
+    case FOURCC_META:
+    case FOURCC_MECO:
+    case FOURCC_STYP:
+    case FOURCC_SIDX:
+    case FOURCC_SSIX:
+    case FOURCC_PRFT:
+      return true;
+    default:
+      // Hex is used to show nonprintable characters and aid in debugging
+      LOG(WARNING) << "Unrecognized top-level box type 0x"
+                   << std::hex << type;
+      return false;
+  }
+}
+
 bool BoxReader::ScanChildren() {
   DCHECK(!scanned_);
   scanned_ = true;

media/mp4/box_reader.h

@@ -91,6 +91,11 @@ class MEDIA_EXPORT BoxReader : public BufferReader {
                                int* box_size,
                                bool* err) WARN_UNUSED_RESULT;
 
+  // Returns true if |type| is recognized to be a top-level box, false
+  // otherwise. This returns true for some boxes which we do not parse.
+  // Helpful in debugging misaligned appends.
+  static bool IsValidTopLevelBox(const FourCC& type);
+
   // Scan through all boxes within the current box, starting at the current
   // buffer position. Must be called before any of the *Child functions work.
   bool ScanChildren() WARN_UNUSED_RESULT;

media/mp4/box_reader_unittest.cc

@@ -14,46 +14,46 @@
 namespace media {
 namespace mp4 {
 
-static const uint8 kTestBox[] = {
-  // Test box containing three children
-  0x00, 0x00, 0x00, 0x40, 't', 'e', 's', 't',
+static const uint8 kSkipBox[] = {
+  // Top-level test box containing three children
+  0x00, 0x00, 0x00, 0x40, 's', 'k', 'i', 'p',
   0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
   0xf9, 0x0a, 0x0b, 0x0c, 0xfd, 0x0e, 0x0f, 0x10,
-  // Ordinary child box
-  0x00, 0x00, 0x00, 0x0c,  'c',  'h',  'l',  'd', 0xde, 0xad, 0xbe, 0xef,
-  // Extended-size child box
-  0x00, 0x00, 0x00, 0x01,  'c',  'h',  'l',  'd',
+  // Ordinary (8-byte header) child box
+  0x00, 0x00, 0x00, 0x0c,  'p',  's',  's',  'h', 0xde, 0xad, 0xbe, 0xef,
+  // Extended-size header child box
+  0x00, 0x00, 0x00, 0x01,  'p',  's',  's',  'h',
   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x14,
   0xfa, 0xce, 0xca, 0xfe,
-  // Empty box
-  0x00, 0x00, 0x00, 0x08,  'm',  'p',  't',  'y',
+  // Empty free box
+  0x00, 0x00, 0x00, 0x08,  'f',  'r',  'e',  'e',
   // Trailing garbage
   0x00 };
 
-struct EmptyBox : Box {
+struct FreeBox : Box {
   virtual bool Parse(BoxReader* reader) OVERRIDE {
     return true;
   }
-  virtual FourCC BoxType() const OVERRIDE { return FOURCC_MPTY; }
+  virtual FourCC BoxType() const OVERRIDE { return FOURCC_FREE; }
 };
 
-struct ChildBox : Box {
+struct PsshBox : Box {
   uint32 val;
 
   virtual bool Parse(BoxReader* reader) OVERRIDE {
     return reader->Read4(&val);
   }
-  virtual FourCC BoxType() const OVERRIDE { return FOURCC_CHLD; }
+  virtual FourCC BoxType() const OVERRIDE { return FOURCC_PSSH; }
 };
 
-struct TestBox : Box {
+struct SkipBox : Box {
   uint8 a, b;
   uint16 c;
   int32 d;
   int64 e;
 
-  std::vector<ChildBox> kids;
-  EmptyBox mpty;
+  std::vector<PsshBox> kids;
+  FreeBox mpty;
 
   virtual bool Parse(BoxReader* reader) OVERRIDE {
     RCHECK(reader->ReadFullBoxHeader() &&
@@ -66,19 +66,19 @@ struct TestBox : Box {
            reader->ReadChildren(&kids) &&
            reader->MaybeReadChild(&mpty);
   }
-  virtual FourCC BoxType() const OVERRIDE { return FOURCC_TEST; }
+  virtual FourCC BoxType() const OVERRIDE { return FOURCC_SKIP; }
 
-  TestBox();
-  ~TestBox();
+  SkipBox();
+  ~SkipBox();
 };
 
-TestBox::TestBox() {}
-TestBox::~TestBox() {}
+SkipBox::SkipBox() {}
+SkipBox::~SkipBox() {}
 
 class BoxReaderTest : public testing::Test {
  protected:
   std::vector<uint8> GetBuf() {
-    return std::vector<uint8>(kTestBox, kTestBox + sizeof(kTestBox));
+    return std::vector<uint8>(kSkipBox, kSkipBox + sizeof(kSkipBox));
   }
 };
 
@@ -90,7 +90,7 @@ TEST_F(BoxReaderTest, ExpectedOperationTest) {
   EXPECT_FALSE(err);
   EXPECT_TRUE(reader.get());
 
-  TestBox box;
+  SkipBox box;
   EXPECT_TRUE(box.Parse(reader.get()));
   EXPECT_EQ(0x01, reader->version());
   EXPECT_EQ(0x020304u, reader->flags());
@@ -129,7 +129,7 @@ TEST_F(BoxReaderTest, InnerTooLongTest) {
   scoped_ptr<BoxReader> reader(
       BoxReader::ReadTopLevelBox(&buf[0], buf.size(), &err));
 
-  TestBox box;
+  SkipBox box;
   EXPECT_FALSE(box.Parse(reader.get()));
 }
 
@@ -137,23 +137,12 @@ TEST_F(BoxReaderTest, WrongFourCCTest) {
   std::vector<uint8> buf = GetBuf();
   bool err;
 
-  // Use an unknown FourCC both on an outer box and an inner one.
+  // Set an unrecognized top-level FourCC.
   buf[5] = 1;
-  buf[28] = 1;
   scoped_ptr<BoxReader> reader(
       BoxReader::ReadTopLevelBox(&buf[0], buf.size(), &err));
-
-  TestBox box;
-  std::vector<ChildBox> kids;
-  // This should still work; the outer box reader doesn't care about the FourCC,
-  // since it assumes you've already examined it before deciding what to parse.
-  EXPECT_TRUE(box.Parse(reader.get()));
-  EXPECT_EQ(0x74017374, reader->type());
-  // Parsing the TestBox should have left the modified inner box unread, which
-  // we collect here.
-  EXPECT_TRUE(reader->ReadAllChildren(&kids));
-  EXPECT_EQ(1u, kids.size());
-  EXPECT_EQ(0xdeadbeef, kids[0].val);
+  EXPECT_FALSE(reader.get());
+  EXPECT_TRUE(err);
 }
 
 TEST_F(BoxReaderTest, ChildrenTest) {
@@ -164,12 +153,12 @@ TEST_F(BoxReaderTest, ChildrenTest) {
 
   EXPECT_TRUE(reader->SkipBytes(16) && reader->ScanChildren());
 
-  EmptyBox mpty;
-  EXPECT_TRUE(reader->ReadChild(&mpty));
-  EXPECT_FALSE(reader->ReadChild(&mpty));
-  EXPECT_TRUE(reader->MaybeReadChild(&mpty));
+  FreeBox free;
+  EXPECT_TRUE(reader->ReadChild(&free));
+  EXPECT_FALSE(reader->ReadChild(&free));
+  EXPECT_TRUE(reader->MaybeReadChild(&free));
 
-  std::vector<ChildBox> kids;
+  std::vector<PsshBox> kids;
 
   EXPECT_TRUE(reader->ReadAllChildren(&kids));
   EXPECT_EQ(2u, kids.size());

media/mp4/fourccs.h

@@ -32,7 +32,9 @@ enum FourCC {
   FOURCC_MDAT = 0x6d646174,
   FOURCC_MDHD = 0x6d646864,
   FOURCC_MDIA = 0x6d646961,
+  FOURCC_MECO = 0x6d65636f,
   FOURCC_MEHD = 0x6d656864,
+  FOURCC_META = 0x6d657461,
   FOURCC_MFHD = 0x6d666864,
   FOURCC_MFRA = 0x6d667261,
   FOURCC_MINF = 0x6d696e66,
@@ -43,6 +45,8 @@ enum FourCC {
   FOURCC_MVEX = 0x6d766578,
   FOURCC_MVHD = 0x6d766864,
   FOURCC_PASP = 0x70617370,
+  FOURCC_PDIN = 0x7064696e,
+  FOURCC_PRFT = 0x70726674,
   FOURCC_PSSH = 0x70737368,
   FOURCC_SAIO = 0x7361696f,
   FOURCC_SAIZ = 0x7361697a,
@@ -54,6 +58,7 @@ enum FourCC {
   FOURCC_SKIP = 0x736b6970,
   FOURCC_SMHD = 0x736d6864,
   FOURCC_SOUN = 0x736f756e,
+  FOURCC_SSIX = 0x73736978,
   FOURCC_STBL = 0x7374626c,
   FOURCC_STCO = 0x7374636f,
   FOURCC_STSC = 0x73747363,
@@ -75,11 +80,6 @@ enum FourCC {
   FOURCC_VIDE = 0x76696465,
   FOURCC_VMHD = 0x766d6864,
   FOURCC_WIDE = 0x77696465,
-
-  // The following are used for testing
-  FOURCC_CHLD = 0x63686c64,
-  FOURCC_MPTY = 0x6d707479,
-  FOURCC_TEST = 0x74657374,
 };
 
 const inline std::string FourCCToString(FourCC fourcc) {

media/mp4/mp4_stream_parser.cc

@@ -462,7 +462,7 @@ bool MP4StreamParser::ReadAndDiscardMDATsUntil(const int64 offset) {
       break;
 
     if (type != FOURCC_MDAT) {
-      DLOG(WARNING) << "Unexpected type while parsing MDATs: "
+      DLOG(WARNING) << "Unexpected box type while parsing MDATs: "
                     << FourCCToString(type);
     }
     mdat_tail_ += box_sz;