Set error on unrecognized top-level BMFF boxes.

media/mp4/box_reader.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/mp4/box_reader.h"
 
 #include <string.h>
 #include <algorithm>
 #include <map>
 #include <set>
 
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
 #include "media/mp4/box_definitions.h"
 #include "media/mp4/rcheck.h"
 
 namespace media {
 namespace mp4 {
 
 Box::~Box() {}
 
 bool BufferReader::Read1(uint8* v) {
   RCHECK(HasBytes(1));
@@ skipping 65 matching lines @@
 
 BoxReader::~BoxReader() {
   if (scanned_ && !children_.empty()) {
     for (ChildMap::iterator itr = children_.begin();
          itr != children_.end(); ++itr) {
       DVLOG(1) << "Skipping unknown box: " << FourCCToString(itr->first);
     }
   }
 }
 
+// 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())) {
+    LOG(WARNING) << "Unrecognized top-level box type 0x"
+                 << std::hex << reader->type();

[acolwell GONE FROM CHROMIUM, 2012/08/02 20:38:18]

Any reason not to use FourCCToString() here like you do in other places?

[strobe_, 2012/08/02 21:27:50]

In the expected case of misregistration, the box type will often contain
unprintable characters. Comment added.

[acolwell GONE FROM CHROMIUM, 2012/08/02 21:40:25]

Ok. Makes sense.

+    *err = true;
+    return NULL;
   }
-  delete reader;
+
+  if (reader->size() <= buf_size)
+    return reader.release();
+
   return NULL;
 }
 
 // static
 bool BoxReader::StartTopLevelBox(const uint8* buf,
                                  const int buf_size,
                                  FourCC* type,
                                  int* box_size,
                                  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) {
+  return (type == FOURCC_FTYP ||

[acolwell GONE FROM CHROMIUM, 2012/08/02 20:38:18]

nit: WDYT about using a switch() here w/ a return true and putting the logging
statement you have above in default: ? That way you'll log for ReadTopLevelBox()
& StartTopLevelBox().

[strobe_, 2012/08/02 21:27:50]

Done.

+          type == FOURCC_PDIN ||
+          type == FOURCC_MOOV ||
+          type == FOURCC_MOOF ||
+          type == FOURCC_MFRA ||
+          type == FOURCC_MDAT ||
+          type == FOURCC_FREE ||
+          type == FOURCC_SKIP ||
+          type == FOURCC_META ||
+          type == FOURCC_MECO ||
+          type == FOURCC_STYP ||
+          type == FOURCC_SIDX ||
+          type == FOURCC_SSIX ||
+          type == FOURCC_PRFT);
+}
+
 bool BoxReader::ScanChildren() {
   DCHECK(!scanned_);
   scanned_ = true;
 
   bool err = false;
   // TODO(strobe): Check or correct for multimap not inserting elements in
   // consistent order.
   while (pos() < size()) {
     BoxReader child(&buf_[pos_], size_ - pos_);
     if (!child.ReadHeader(&err)) break;
@@ skipping 56 matching lines @@
   }
 
   // Note that the pos_ head has advanced to the byte immediately after the
   // header, which is where we want it.
   size_ = size;
   return true;
 }
 
 }  // namespace mp4
 }  // namespace media