From 6509306bbe50f3b7481672eb1f64b1bd2475a257 Mon Sep 17 00:00:00 2001
From: Seigo Nonaka <nona@google.com>
Date: Fri, 7 Jul 2017 11:39:58 -0700
Subject: Reject unsorted cmap entries. DO NOT MERGE

addRange assumes the passing ranges are sorted in ascending order which
is a part of OpenType spec, but bad fonts can pass arbitrary ranges.
Now, addRange rejects invalid input and stop using such bad fonts.

Bug: 32178311
Test: minikin_tests
Test: bit CtsGraphicsTestCases:.TypefaceTest
Change-Id: Ice845a1206e1c9da08ea20c7b56fde2e6ec8b673
(cherry picked from commit 0c943b002dfed4c19196a4d162737057f8ed5a56)
(cherry picked from commit dbe6bb0c2b612889c2b1a3e37a77b93683c76e03)
CVE-2017-0755
---
 libs/minikin/CmapCoverage.cpp | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/libs/minikin/CmapCoverage.cpp b/libs/minikin/CmapCoverage.cpp
index a4503af..eb46c41 100644
--- a/libs/minikin/CmapCoverage.cpp
+++ b/libs/minikin/CmapCoverage.cpp
@@ -37,15 +37,25 @@ static uint32_t readU32(const uint8_t* data, size_t offset) {
         ((uint32_t)data[offset + 2]) << 8 | ((uint32_t)data[offset + 3]);
 }
 
-static void addRange(vector<uint32_t> &coverage, uint32_t start, uint32_t end) {
+// The start must be larger than or equal to coverage.back() if coverage is not empty.
+// Returns true if the range is appended. Otherwise returns false as an error.
+static bool addRange(vector<uint32_t> &coverage, uint32_t start, uint32_t end) {
 #ifdef VERBOSE_DEBUG
     ALOGD("adding range %d-%d\n", start, end);
 #endif
     if (coverage.empty() || coverage.back() < start) {
         coverage.push_back(start);
         coverage.push_back(end);
-    } else {
+        return true;
+    } else if (coverage.back() == start) {
         coverage.back() = end;
+        return true;
+    } else {
+        // Reject unordered range input since SparseBitSet assumes that the given range vector is
+        // sorted. OpenType specification says cmap entries are sorted in order of code point
+        // values, thus for OpenType compliant font files, we don't reach here.
+        android_errorWriteLog(0x534e4554, "32178311");
+        return false;
     }
 }
 
@@ -74,11 +84,15 @@ static bool getCoverageFormat4(vector<uint32_t>& coverage, const uint8_t* data,
         if (rangeOffset == 0) {
             uint32_t delta = readU16(data, kHeaderSize + 2 * (2 * segCount + i));
             if (((end + delta) & 0xffff) > end - start) {
-                addRange(coverage, start, end + 1);
+                if (!addRange(coverage, start, end + 1)) {
+                    return false;
+                }
             } else {
                 for (uint32_t j = start; j < end + 1; j++) {
                     if (((j + delta) & 0xffff) != 0) {
-                        addRange(coverage, j, j + 1);
+                        if (!addRange(coverage, j, j + 1)) {
+                            return false;
+                        }
                     }
                 }
             }
@@ -92,7 +106,9 @@ static bool getCoverageFormat4(vector<uint32_t>& coverage, const uint8_t* data,
                 }
                 uint32_t glyphId = readU16(data, actualRangeOffset);
                 if (glyphId != 0) {
-                    addRange(coverage, j, j + 1);
+                    if (!addRange(coverage, j, j + 1)) {
+                        return false;
+                    }
                 }
             }
         }
@@ -126,7 +142,9 @@ static bool getCoverageFormat12(vector<uint32_t>& coverage, const uint8_t* data,
             android_errorWriteLog(0x534e4554, "26413177");
             return false;
         }
-        addRange(coverage, start, end + 1);  // file is inclusive, vector is exclusive
+        if (!addRange(coverage, start, end + 1)) {  // file is inclusive, vector is exclusive
+            return false;
+        }
     }
     return true;
 }
-- 
cgit v1.2.3