Fix memory corruption issues in TextLayoutCache. (DO NOT MERGE)

Ensure log_clusters array is big enough.
Bug: 5714171

Explicitly handle the cases where the entire string or a single
run might have a length of 0.
Harfbuzz assumes the length of the item is at least 1.
If the length is zero, then it will clobber memory at index -1
into the log_clusters array.
Bug: 5705479

Change-Id: If28a9866221081f69973c1d12d7fe0cf8db2edd0

1 files changed, 18 insertions, 5 deletions

diff --git a/core/jni/android/graphics/TextLayoutCache.cpp b/core/jni/android/graphics/TextLayoutCache.cpp
index 7076e2a..510e541 100644
--- a/core/jni/android/graphics/TextLayoutCache.cpp
+++ b/core/jni/android/graphics/TextLayoutCache.cpp
@@ -368,9 +368,6 @@ void TextLayoutCacheValue::initShaperItem(HB_ShaperItem& shaperItem, HB_FontRec*
     // padding and fallback if we find that we are wrong.
     createGlyphArrays(shaperItem, (contextCount + 2) * 2);
 
-    // Create log clusters array
-    shaperItem.log_clusters = new unsigned short[contextCount];
-
     // Set the string properties
     shaperItem.string = chars;
     shaperItem.stringLength = contextCount;
@@ -378,7 +375,6 @@ void TextLayoutCacheValue::initShaperItem(HB_ShaperItem& shaperItem, HB_FontRec*
 
 void TextLayoutCacheValue::freeShaperItem(HB_ShaperItem& shaperItem) {
     deleteGlyphArrays(shaperItem);
-    delete[] shaperItem.log_clusters;
     HB_FreeFace(shaperItem.face);
 }
 
@@ -392,6 +388,7 @@ void TextLayoutCacheValue::shapeRun(HB_ShaperItem& shaperItem, size_t start, siz
     shaperItem.item.script = isRTL ? HB_Script_Arabic : HB_Script_Common;
 
     // Shape
+    assert(shaperItem.item.length > 0); // Harfbuzz will overwrite other memory if length is 0.
     while (!HB_ShapeItem(&shaperItem)) {
         // We overflowed our arrays. Resize and retry.
         // HB_ShapeItem fills in shaperItem.num_glyphs with the needed size.
@@ -404,6 +401,10 @@ void TextLayoutCacheValue::computeValuesWithHarfbuzz(SkPaint* paint, const UChar
         size_t start, size_t count, size_t contextCount, int dirFlags,
         Vector<jfloat>* const outAdvances, jfloat* outTotalAdvance,
         Vector<jchar>* const outGlyphs) {
+        if (!count) {
+            *outTotalAdvance = 0;
+            return;
+        }
 
         UBiDiLevel bidiReq = 0;
         bool forceLTR = false;
@@ -544,6 +545,10 @@ void TextLayoutCacheValue::computeRunValuesWithHarfbuzz(HB_ShaperItem& shaperIte
         size_t start, size_t count, bool isRTL,
         Vector<jfloat>* const outAdvances, jfloat* outTotalAdvance,
         Vector<jchar>* const outGlyphs) {
+    if (!count) {
+        *outTotalAdvance = 0;
+        return;
+    }
 
     shapeRun(shaperItem, start, count, isRTL);
 
@@ -607,14 +612,22 @@ void TextLayoutCacheValue::deleteGlyphArrays(HB_ShaperItem& shaperItem) {
     delete[] shaperItem.attributes;
     delete[] shaperItem.advances;
     delete[] shaperItem.offsets;
+    delete[] shaperItem.log_clusters;
 }
 
 void TextLayoutCacheValue::createGlyphArrays(HB_ShaperItem& shaperItem, int size) {
+    shaperItem.num_glyphs = size;
+
+    // These arrays are all indexed by glyph
     shaperItem.glyphs = new HB_Glyph[size];
     shaperItem.attributes = new HB_GlyphAttributes[size];
     shaperItem.advances = new HB_Fixed[size];
     shaperItem.offsets = new HB_FixedPoint[size];
-    shaperItem.num_glyphs = size;
+
+    // Although the log_clusters array is indexed by character, Harfbuzz expects that
+    // it is big enough to hold one element per glyph.  So we allocate log_clusters along
+    // with the other glyph arrays above.
+    shaperItem.log_clusters = new unsigned short[size];
 }
 
 } // namespace android