Double free in ~SkPictureData()

On subpicture parsing failures we clean up all fPictureRefs entries
*and* delete the array itself.  But the destructor also deletes the
array => double free.

Alternatively, we can set fPictureCount to the number of successfully
parsed pictures such that the destructor handles all the cleanup.

BUG=515228
R=reed@google.com,mtklein@google.com

Review URL: https://codereview.chromium.org/1264503011

1 files changed, 6 insertions, 18 deletions

diff --git a/src/core/SkPictureData.cpp b/src/core/SkPictureData.cpp
index fc4fdb8230..4f3ac37066 100644
--- a/src/core/SkPictureData.cpp
+++ b/src/core/SkPictureData.cpp
@@ -373,26 +373,14 @@ bool SkPictureData::parseStreamTag(SkStream* stream,
             }
         } break;
         case SK_PICT_PICTURE_TAG: {
-            fPictureCount = size;
-            fPictureRefs = SkNEW_ARRAY(const SkPicture*, fPictureCount);
-            bool success = true;
-            int i = 0;
-            for ( ; i < fPictureCount; i++) {
+            fPictureCount = 0;
+            fPictureRefs = SkNEW_ARRAY(const SkPicture*, size);
+            for (uint32_t i = 0; i < size; i++) {
                 fPictureRefs[i] = SkPicture::CreateFromStream(stream, proc);
-                if (NULL == fPictureRefs[i]) {
-                    success = false;
-                    break;
-                }
-            }
-            if (!success) {
-                // Delete all of the pictures that were already created (up to but excluding i):
-                for (int j = 0; j < i; j++) {
-                    fPictureRefs[j]->unref();
+                if (!fPictureRefs[i]) {
+                    return false;
                 }
-                // Delete the array
-                SkDELETE_ARRAY(fPictureRefs);
-                fPictureCount = 0;
-                return false;
+                fPictureCount++;
             }
         } break;
         case SK_PICT_BUFFER_SIZE_TAG: {