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)
diff --git a/libs/minikin/CmapCoverage.cpp b/libs/minikin/CmapCoverage.cpp
index 9659d0c..c56d07c 100644
--- a/libs/minikin/CmapCoverage.cpp
+++ b/libs/minikin/CmapCoverage.cpp
@@ -49,15 +49,25 @@
((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;
}
}
@@ -109,7 +119,7 @@
// No ranges left in rRanges. Just put all remaining ranges in lRanges.
do {
Range r = getRange(lRanges, li);
- addRange(out, r.start, r.end);
+ addRange(out, r.start, r.end); // Input is sorted. Never returns false.
li += 2;
} while (li < lsize);
break;
@@ -117,17 +127,17 @@
// No ranges left in lRanges. Just put all remaining ranges in rRanges.
do {
Range r = getRange(rRanges, ri);
- addRange(out, r.start, r.end);
+ addRange(out, r.start, r.end); // Input is sorted. Never returns false.
ri += 2;
} while (ri < rsize);
break;
} else if (!Range::intersects(left, right)) {
// No intersection. Add smaller range.
if (left.start < right.start) {
- addRange(out, left.start, left.end);
+ addRange(out, left.start, left.end); // Input is sorted. Never returns false.
li += 2;
} else {
- addRange(out, right.start, right.end);
+ addRange(out, right.start, right.end); // Input is sorted. Never returns false.
ri += 2;
}
} else {
@@ -147,7 +157,7 @@
right = getRange(rRanges, ri);
}
}
- addRange(out, merged.start, merged.end);
+ addRange(out, merged.start, merged.end); // Input is sorted. Never returns false.
}
}
@@ -179,11 +189,15 @@
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;
+ }
}
}
}
@@ -197,7 +211,9 @@
}
uint32_t glyphId = readU16(data, actualRangeOffset);
if (glyphId != 0) {
- addRange(coverage, j, j + 1);
+ if (!addRange(coverage, j, j + 1)) {
+ return false;
+ }
}
}
}
@@ -238,10 +254,11 @@
}
if (end > MAX_UNICODE_CODE_POINT) {
// file is inclusive, vector is exclusive
- addRange(coverage, start, MAX_UNICODE_CODE_POINT + 1);
- return true;
+ return addRange(coverage, start, MAX_UNICODE_CODE_POINT + 1);
}
- 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;
}
@@ -306,7 +323,9 @@
for (uint32_t i = 0; i < numRecords; ++i) {
const size_t recordOffset = kHeaderSize + kUVSMappingRecordSize * i;
const uint32_t codePoint = readU24(nonDefaultUVSTable, recordOffset);
- addRange(rangesFromNonDefaultUVSTable, codePoint, codePoint + 1);
+ if (!addRange(rangesFromNonDefaultUVSTable, codePoint, codePoint + 1)) {
+ return false;
+ }
}
}
@@ -342,7 +361,9 @@
// without variation selectors. We need to check the default glyph availability and
// exclude the codepoint if it is not supported by defualt cmap table.
if (baseCoverage.get(cp)) {
- addRange(rangesFromDefaultUVSTable, cp, cp + 1 /* exclusive */);
+ if (!addRange(rangesFromDefaultUVSTable, cp, cp + 1 /* exclusive */)) {
+ return false;
+ }
}
}
}
diff --git a/tests/unittest/CmapCoverageTest.cpp b/tests/unittest/CmapCoverageTest.cpp
index edf1ca1..fe2d7ba 100644
--- a/tests/unittest/CmapCoverageTest.cpp
+++ b/tests/unittest/CmapCoverageTest.cpp
@@ -314,6 +314,24 @@
EXPECT_EQ(0U, coverage.length());
EXPECT_TRUE(vsTables.empty());
}
+ {
+ SCOPED_TRACE("Reversed range");
+ std::vector<uint8_t> cmap = CmapBuilder::buildSingleFormat4Cmap(0, 0, std::vector<uint16_t>(
+ {'b', 'b', 'a', 'a'}));
+
+ SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
+ EXPECT_EQ(0U, coverage.length());
+ EXPECT_TRUE(vsTables.empty());
+ }
+ {
+ SCOPED_TRACE("Reversed range - partially readable");
+ std::vector<uint8_t> cmap = CmapBuilder::buildSingleFormat4Cmap(0, 0, std::vector<uint16_t>(
+ { 'a', 'a', 'c', 'c', 'b', 'b'}));
+
+ SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
+ EXPECT_EQ(0U, coverage.length());
+ EXPECT_TRUE(vsTables.empty());
+ }
}
TEST(CmapCoverageTest, SingleFormat4) {
@@ -525,6 +543,22 @@
EXPECT_EQ(0U, coverage.length());
EXPECT_TRUE(vsTables.empty());
}
+ {
+ SCOPED_TRACE("Reversed range");
+ std::vector<uint8_t> cmap = CmapBuilder::buildSingleFormat12Cmap(
+ 0, 0, std::vector<uint32_t>({'b', 'b', 'a', 'a'}));
+ SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
+ EXPECT_EQ(0U, coverage.length());
+ EXPECT_TRUE(vsTables.empty());
+ }
+ {
+ SCOPED_TRACE("Reversed range - partially readable");
+ std::vector<uint8_t> cmap = CmapBuilder::buildSingleFormat12Cmap(
+ 0, 0, std::vector<uint32_t>({'a', 'a', 'c', 'c', 'b', 'b'}));
+ SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
+ EXPECT_EQ(0U, coverage.length());
+ EXPECT_TRUE(vsTables.empty());
+ }
}
TEST(CmapCoverageTest, TableSelection_Priority) {
@@ -804,7 +838,7 @@
}
TEST(CmapCoverageTest, TableSelection_brokenVSTable) {
- std::vector<uint8_t> cmap12Table = buildCmapFormat12Table(std::vector<uint32_t>({'a', 'a'}));
+ std::vector<uint8_t> cmap12Table = buildCmapFormat12Table(std::vector<uint32_t>({'a', 'z'}));
{
SCOPED_TRACE("Too small cmap size");
std::vector<uint8_t> vsTable = buildCmapFormat14Table(std::vector<VariationSelectorRecord>({
@@ -898,6 +932,62 @@
SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
ASSERT_TRUE(vsTables.empty());
}
+ {
+ SCOPED_TRACE("Reversed range in default UVS table");
+ std::vector<uint8_t> vsTable = buildCmapFormat14Table(std::vector<VariationSelectorRecord>({
+ { 0xFE0F, { 'b', 'b', 'a', 'a' }, { } }
+ }));
+ 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());
+ }
+ {
+ SCOPED_TRACE("Reversed range in default UVS table - partially readable");
+ std::vector<uint8_t> vsTable = buildCmapFormat14Table(std::vector<VariationSelectorRecord>({
+ { 0xFE0F, { 'a', 'a', 'c', 'c', 'b', 'b' }, { } }
+ }));
+ 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());
+ }
+ {
+ SCOPED_TRACE("Reversed mapping entries in non default UVS table");
+ std::vector<uint8_t> vsTable = buildCmapFormat14Table(std::vector<VariationSelectorRecord>({
+ { 0xFE0F, { }, { 'b', 'a' } }
+ }));
+ 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());
+ }
+ {
+ SCOPED_TRACE("Reversed mapping entries in non default UVS table");
+ std::vector<uint8_t> vsTable = buildCmapFormat14Table(std::vector<VariationSelectorRecord>({
+ { 0xFE0F, { }, { 'a', 'c', 'b' } }
+ }));
+ 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) {
