commit 3056f04d293bd16e56cc72e10edd060b8c1ca0f5
author Seigo Nonaka <nona@google.com> Wed Jan 17 13:18:36 2018 -0800
committer android-build-team Robot <android-build-team-robot@google.com> Wed Feb 14 17:17:45 2018 +0000
tree f548e640ad238d6f0a7cd3f677f08e77c5cb92e3
parent 6873eb9c1229f1e5a66e7b73a62fca05a2174a3d

Fix OOB read due to integer overflow

This only happens on 32bit system.

Bug: 70808908
Test: minikin_tests
Change-Id: Id394e0ccf83bddd2db9957386dc3ed892feae8a5
(cherry picked from commit 2f88f76f257d046066ede752156968aff712114b)

libs/minikin/CmapCoverage.cpp

diff --git a/libs/minikin/CmapCoverage.cpp b/libs/minikin/CmapCoverage.cpp
index c6318f5..78e7b0d 100644
--- a/libs/minikin/CmapCoverage.cpp
+++ b/libs/minikin/CmapCoverage.cpp
@@ -319,8 +319,12 @@
         if (nonDefaultUVSTableRemaining < kHeaderSize) {
             return false;
         }
-        const uint32_t numRecords = readU32(nonDefaultUVSTable, 0);
-        if (numRecords * kUVSMappingRecordSize + kHeaderSize > nonDefaultUVSTableRemaining) {
+        const uint64_t numRecords = readU32(nonDefaultUVSTable, 0);
+        const uint64_t sizeToRead = numRecords * kUVSMappingRecordSize + kHeaderSize;
+        if (sizeToRead > nonDefaultUVSTableRemaining) {
+            if (sizeToRead > UINT_MAX) {
+                android_errorWriteLog(0x534e4554, "70808908");
+            }
             return false;
         }
         for (uint32_t i = 0; i < numRecords; ++i) {
@@ -347,8 +351,12 @@
         if (defaultUVSTableRemaining < kHeaderSize) {
             return false;
         }
-        const uint32_t numRecords = readU32(defaultUVSTable, 0);
-        if (numRecords * kUnicodeRangeRecordSize + kHeaderSize > defaultUVSTableRemaining) {
+        const uint64_t numRecords = readU32(defaultUVSTable, 0);
+        const uint64_t sizeToRead = numRecords * kUnicodeRangeRecordSize + kHeaderSize;
+        if (sizeToRead > defaultUVSTableRemaining) {
+            if (sizeToRead > UINT_MAX) {
+                android_errorWriteLog(0x534e4554, "70808908");
+            }
             return false;
         }
 
@@ -392,8 +400,12 @@
         return;
     }
 
-    uint32_t numRecords = readU32(data, kNumRecordOffset);
-    if (numRecords == 0 || kHeaderSize + kRecordSize * numRecords > length) {
+    const uint64_t numRecords = readU32(data, kNumRecordOffset);
+    const uint64_t sizeToRead = kHeaderSize + kRecordSize * numRecords;
+    if (numRecords == 0 || sizeToRead > length) {
+        if (sizeToRead > UINT_MAX) {
+            android_errorWriteLog(0x534e4554, "70808908");
+        }
         return;
     }
 

tests/unittest/CmapCoverageTest.cpp

diff --git a/tests/unittest/CmapCoverageTest.cpp b/tests/unittest/CmapCoverageTest.cpp
index fe2d7ba..590a363 100644
--- a/tests/unittest/CmapCoverageTest.cpp
+++ b/tests/unittest/CmapCoverageTest.cpp
@@ -988,6 +988,54 @@
         SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
         ASSERT_TRUE(vsTables.empty());
     }
+    {
+        // http://b/70808908
+        SCOPED_TRACE("OOB access due to integer overflow in non default UVS table");
+        std::vector<uint8_t> vsTable = buildCmapFormat14Table(
+                std::vector<VariationSelectorRecord>({{0xFE0F, {'a', 'a'}, {'b'}}}));
+        // 6 is the offset of the numRecords in the Cmap format14 subtable header.
+        writeU32(0x1745d174 /* 2^32 / kRecordSize(=11) */, vsTable.data(), 6);
+        CmapBuilder builder(2);
+        builder.appendTable(3, 1, cmap12Table);
+        builder.appendTable(VS_PLATFORM_ID, VS_ENCODING_ID, vsTable);
+        std::vector<uint8_t> cmap = builder.build();
+
+        std::vector<std::unique_ptr<SparseBitSet>> vsTables;
+        SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
+        ASSERT_TRUE(vsTables.empty());
+    }
+    {
+        // http://b/70808908
+        SCOPED_TRACE("OOB access due to integer overflow in non default UVS table");
+        std::vector<uint8_t> vsTable = buildCmapFormat14Table(
+                std::vector<VariationSelectorRecord>({{0xFE0F, {'a', 'a'}, {'b'}}}));
+        // 29 is the offset of the numUVSMappings in the fist non defulat UVS table.
+        writeU32(0x33333333 /* 2^32 / kUVSMappingRecordSize(=5) */, vsTable.data(), 29);
+        CmapBuilder builder(2);
+        builder.appendTable(3, 1, cmap12Table);
+        builder.appendTable(VS_PLATFORM_ID, VS_ENCODING_ID, vsTable);
+        std::vector<uint8_t> cmap = builder.build();
+
+        std::vector<std::unique_ptr<SparseBitSet>> vsTables;
+        SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
+        ASSERT_TRUE(vsTables.empty());
+    }
+    {
+        // http://b/70808908
+        SCOPED_TRACE("OOB access due to integer overflow in default UVS table");
+        std::vector<uint8_t> vsTable = buildCmapFormat14Table(
+                std::vector<VariationSelectorRecord>({{0xFE0F, {'a', 'a'}, {'b'}}}));
+        // 21 is the offset of the numUnicodeValueRanges in the fist defulat UVS table.
+        writeU32(0x40000000 /* 2^32 / kUnicodeRangeRecordSize(=4) */, vsTable.data(), 21);
+        CmapBuilder builder(2);
+        builder.appendTable(3, 1, cmap12Table);
+        builder.appendTable(VS_PLATFORM_ID, VS_ENCODING_ID, vsTable);
+        std::vector<uint8_t> cmap = builder.build();
+
+        std::vector<std::unique_ptr<SparseBitSet>> vsTables;
+        SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
+        ASSERT_TRUE(vsTables.empty());
+    }
 }
 
 TEST(CmapCoverageTest, TableSelection_brokenVSTable_bestEffort) {