Merge pull request #60 from leizleiz/leizleiz-crbug

Fix bugs found by Chromium's sfntly fuzzer and other bug reporters

8 files changed, 57 insertions, 23 deletions

diff --git a/cpp/src/sample/subtly/font_assembler.cc b/cpp/src/sample/subtly/font_assembler.cc
index 2f7cd11..4717512 100644
--- a/cpp/src/sample/subtly/font_assembler.cc
+++ b/cpp/src/sample/subtly/font_assembler.cc
@@ -211,6 +211,8 @@ bool FontAssembler::AssembleGlyphAndLocaTables() {
     // If there are missing glyphs between the last glyph_id and the
     // current resolved_glyph_id, since the LOCA table needs to have the same
     // size, the offset is kept the same.
+    loca_list.resize(std::max(loca_list.size(),
+                     static_cast<size_t>(resolved_glyph_id + 2)));
     for (int32_t i = last_glyph_id + 1; i <= resolved_glyph_id; ++i)
       loca_list[i] = last_offset;
     last_offset += length;
diff --git a/cpp/src/sfntly/data/readable_font_data.cc b/cpp/src/sfntly/data/readable_font_data.cc
index df33f9a..07a0db6 100644
--- a/cpp/src/sfntly/data/readable_font_data.cc
+++ b/cpp/src/sfntly/data/readable_font_data.cc
@@ -111,7 +111,9 @@ int32_t ReadableFontData::ReadShort(int32_t index) {
   int32_t b2 = ReadUByte(index + 1);
   if (b2 < 0)
     return kInvalidShort;
-  return ((b1 << 8 | b2) << 16) >> 16;
+
+  uint32_t result = static_cast<uint32_t>(b1) << 8 | b2;
+  return static_cast<int32_t>(result << 16) >> 16;
 }
 
 int32_t ReadableFontData::ReadUInt24(int32_t index) {
@@ -170,10 +172,19 @@ int64_t ReadableFontData::ReadULongLE(int32_t index) {
 }
 
 int32_t ReadableFontData::ReadLong(int32_t index) {
-  return ReadByte(index) << 24 |
-         ReadUByte(index + 1) << 16 |
-         ReadUByte(index + 2) << 8 |
-         ReadUByte(index + 3);
+  int32_t b1 = ReadByte(index);
+  if (b1 == kInvalidByte)
+    return kInvalidLong;
+  int32_t b2 = ReadUByte(index + 1);
+  if (b2 < 0)
+    return kInvalidLong;
+  int32_t b3 = ReadUByte(index + 2);
+  if (b3 < 0)
+    return kInvalidLong;
+  int32_t b4 = ReadUByte(index + 3);
+  if (b4 < 0)
+    return kInvalidLong;
+  return static_cast<uint32_t>(b1) << 24 | b2 << 16 | b3 << 8 | b4;
 }
 
 int32_t ReadableFontData::ReadFixed(int32_t index) {
@@ -181,7 +192,13 @@ int32_t ReadableFontData::ReadFixed(int32_t index) {
 }
 
 int64_t ReadableFontData::ReadDateTimeAsLong(int32_t index) {
-  return (int64_t)ReadULong(index) << 32 | ReadULong(index + 4);
+  int32_t high = ReadULong(index);
+  if (high == kInvalidUnsigned)
+    return kInvalidLongDateTime;
+  int32_t low = ReadULong(index + 4);
+  if (low == kInvalidUnsigned)
+    return kInvalidLongDateTime;
+  return (int64_t)high << 32 | low;
 }
 
 int32_t ReadableFontData::ReadFWord(int32_t index) {
diff --git a/cpp/src/sfntly/data/readable_font_data.h b/cpp/src/sfntly/data/readable_font_data.h
index 003d494..37a0918 100644
--- a/cpp/src/sfntly/data/readable_font_data.h
+++ b/cpp/src/sfntly/data/readable_font_data.h
@@ -53,7 +53,9 @@ class ReadableFontData : public FontData,
 
   static const int32_t kInvalidByte = 128;
   static const int32_t kInvalidShort = 32768;
+  static const int32_t kInvalidLong = 0xffffffff;
   static const int32_t kInvalidUnsigned = -1;
+  static const int64_t kInvalidLongDateTime = -1;
 
   static CALLER_ATTACH ReadableFontData* CreateReadableFontData(ByteVector* b);
 
@@ -122,7 +124,7 @@ class ReadableFontData : public FontData,
 
   // Read the ULONG at the given index.
   // @param index index into the font data
-  // @return the ULONG
+  // @return the ULONG; kInvalidUnsigned if outside the bounds of the font data
   // @throws IndexOutOfBoundsException if index is outside the FontData's range
   virtual int64_t ReadULong(int32_t index);
 
@@ -140,7 +142,7 @@ class ReadableFontData : public FontData,
 
   // Read the LONG at the given index.
   // @param index index into the font data
-  // @return the LONG
+  // @return the LONG; kInvalidLong if outside the bounds of the font data
   // @throws IndexOutOfBoundsException if index is outside the FontData's range
   virtual int32_t ReadLong(int32_t index);
 
@@ -152,7 +154,8 @@ class ReadableFontData : public FontData,
 
   // Read the LONGDATETIME at the given index.
   // @param index index into the font data
-  // @return the LONGDATETIME
+  // @return the LONGDATETIME; kInvalidLongDateTime if outside the bounds of the
+  // font data
   // @throws IndexOutOfBoundsException if index is outside the FontData's range
   virtual int64_t ReadDateTimeAsLong(int32_t index);
 
diff --git a/cpp/src/sfntly/font.cc b/cpp/src/sfntly/font.cc
index cfb956f..0c268cc 100644
--- a/cpp/src/sfntly/font.cc
+++ b/cpp/src/sfntly/font.cc
@@ -393,14 +393,18 @@ void Font::Builder::BuildTablesFromBuilders(Font* font,
 }
 
 static Table::Builder* GetBuilder(TableBuilderMap* builder_map, int32_t tag) {
-  if (builder_map) {
-    TableBuilderMap::iterator target = builder_map->find(tag);
-    if (target != builder_map->end()) {
-      return target->second.p_;
-    }
-  }
+  if (!builder_map)
+    return NULL;
 
-  return NULL;
+  TableBuilderMap::iterator target = builder_map->find(tag);
+  if (target == builder_map->end())
+    return NULL;
+
+  Table::Builder* builder = target->second.p_;
+  if (!builder || !builder->InternalReadData())
+    return NULL;
+
+  return builder;
 }
 
 void Font::Builder::InterRelateBuilders(TableBuilderMap* builder_map) {
diff --git a/cpp/src/sfntly/table/core/font_header_table.cc b/cpp/src/sfntly/table/core/font_header_table.cc
index 60015ca..a848afd 100644
--- a/cpp/src/sfntly/table/core/font_header_table.cc
+++ b/cpp/src/sfntly/table/core/font_header_table.cc
@@ -239,7 +239,10 @@ void FontHeaderTable::Builder::SetFontDirectionHint(int32_t hint) {
 }
 
 int32_t FontHeaderTable::Builder::IndexToLocFormat() {
-  return down_cast<FontHeaderTable*>(GetTable())->IndexToLocFormat();
+  Table* table = GetTable();
+  if (!table)
+    return IndexToLocFormat::kInvalidOffset;
+  return down_cast<FontHeaderTable*>(table)->IndexToLocFormat();
 }
 
 void FontHeaderTable::Builder::SetIndexToLocFormat(int32_t format) {
diff --git a/cpp/src/sfntly/table/core/font_header_table.h b/cpp/src/sfntly/table/core/font_header_table.h
index 841955b..4851775 100644
--- a/cpp/src/sfntly/table/core/font_header_table.h
+++ b/cpp/src/sfntly/table/core/font_header_table.h
@@ -24,6 +24,7 @@ namespace sfntly {
 
 struct IndexToLocFormat {
   enum {
+    kInvalidOffset = -1,
     kShortOffset = 0,
     kLongOffset = 1
   };
diff --git a/cpp/src/sfntly/table/core/name_table.cc b/cpp/src/sfntly/table/core/name_table.cc
index 8d2f64f..c9fe468 100644
--- a/cpp/src/sfntly/table/core/name_table.cc
+++ b/cpp/src/sfntly/table/core/name_table.cc
@@ -469,12 +469,14 @@ int32_t NameTable::NameId(int32_t index) {
 
 void NameTable::NameAsBytes(int32_t index, ByteVector* b) {
   assert(b);
-  int32_t length = NameLength(index);
   b->clear();
+
+  int32_t length = NameLength(index);
+  if (length <= 0)
+    return;
+
   b->resize(length);
-  if (length > 0) {
-    data_->ReadBytes(NameOffset(index), &((*b)[0]), 0, length);
-  }
+  data_->ReadBytes(NameOffset(index), &((*b)[0]), 0, length);
 }
 
 void NameTable::NameAsBytes(int32_t platform_id,
diff --git a/cpp/src/sfntly/table/table_based_table_builder.cc b/cpp/src/sfntly/table/table_based_table_builder.cc
index b505704..51a5a3b 100644
--- a/cpp/src/sfntly/table/table_based_table_builder.cc
+++ b/cpp/src/sfntly/table/table_based_table_builder.cc
@@ -60,8 +60,10 @@ TableBasedTableBuilder::TableBasedTableBuilder(Header* header)
 }
 
 Table* TableBasedTableBuilder::GetTable() {
-  if (table_ == NULL) {
-    table_.Attach(down_cast<Table*>(SubBuildTable(InternalReadData())));
+  if (!table_) {
+    ReadableFontData* data = InternalReadData();
+    if (data)
+      table_.Attach(down_cast<Table*>(SubBuildTable(data)));
   }
   return table_;
 }