commit 94d4d3e20b8be29233bc7056ed3b8b36def3e98a
author Florin Malita <fmalita@chromium.org> Mon Jun 18 13:10:51 2018 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Mon Jun 18 17:33:18 2018 +0000
tree ed3725d2ca39707cddefeb4d7493ad3228e4cdb7
parent 0333854e552ccfc810dc74d74636692f1d0da67e

[skottie] Fix OOB access in Parse<SkPoint>

SkJSON requires valid array indices, so callers must guard against
out-of-bounds conditions explicitly.

Bug: oss-fuzz:8956
Change-Id: I50b96b088e44a4c1a569e6911d4be5d75799b464
Reviewed-on: https://skia-review.googlesource.com/135445
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Kevin Lubick <kjlubick@google.com>

BUILD.gn

diff --git a/BUILD.gn b/BUILD.gn
index 2a832a5..1691936 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -1385,6 +1385,7 @@
       ":flags",
       ":skia",
       ":tool_utils",
+      "modules/skottie:tests",
       "modules/sksg:tests",
       "//third_party/libpng",
       "//third_party/zlib",

modules/skottie/BUILD.gn

diff --git a/modules/skottie/BUILD.gn b/modules/skottie/BUILD.gn
index dff355c..9442592 100644
--- a/modules/skottie/BUILD.gn
+++ b/modules/skottie/BUILD.gn
@@ -40,6 +40,26 @@
   }
 }
 
+source_set("tests") {
+  if (skia_enable_skottie) {
+    testonly = true
+
+    configs += [
+      "../..:skia_private",
+      "../..:tests_config",
+    ]
+    sources = [
+      "src/SkottieTest.cpp",
+    ]
+
+    deps = [
+      ":skottie",
+      "../..:gpu_tool_utils",
+      "../..:skia",
+    ]
+  }
+}
+
 source_set("fuzz") {
   if (skia_enable_skottie) {
     testonly = true

modules/skottie/src/SkottieJson.cpp

diff --git a/modules/skottie/src/SkottieJson.cpp b/modules/skottie/src/SkottieJson.cpp
index 4f23939..186cd42 100644
--- a/modules/skottie/src/SkottieJson.cpp
+++ b/modules/skottie/src/SkottieJson.cpp
@@ -84,8 +84,12 @@
     const auto& jvy = ov["y"];
 
     // Some BM versions seem to store x/y as single-element arrays.
-    return Parse<SkScalar>(jvx.is<ArrayValue>() ? jvx.as<ArrayValue>()[0] : jvx, &pt->fX)
-        && Parse<SkScalar>(jvy.is<ArrayValue>() ? jvy.as<ArrayValue>()[0] : jvy, &pt->fY);
+    // TODO: We should be able to check size == 1 below, or just delegate to Parse<SkScalar>,
+    //       but that change introduces diffs.  Investigate.
+    const ArrayValue* jvxa = jvx;
+    const ArrayValue* jvya = jvy;
+    return Parse<SkScalar>(jvxa && jvxa->size() > 0 ? (*jvxa)[0] : jvx, &pt->fX)
+        && Parse<SkScalar>(jvya && jvya->size() > 0 ? (*jvya)[0] : jvy, &pt->fY);
 }
 
 template <>

modules/skottie/src/SkottieTest.cpp

diff --git a/modules/skottie/src/SkottieTest.cpp b/modules/skottie/src/SkottieTest.cpp
new file mode 100644
index 0000000..1c4cc07
--- /dev/null
+++ b/modules/skottie/src/SkottieTest.cpp
@@ -0,0 +1,23 @@
+/*
+ * Copyright 2018 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "Skottie.h"
+#include "SkStream.h"
+
+#include "Test.h"
+
+DEF_TEST(Skottie_OssFuzz8956, reporter) {
+    static constexpr const char json[] =
+        "{\"v\":\" \",\"fr\":3,\"w\":4,\"h\":3,\"layers\":[{\"ty\": 1, \"sw\": 10, \"sh\": 10,"
+            " \"sc\":\"#ffffff\", \"ks\":{\"o\":{\"a\": true, \"k\":"
+            " [{\"t\": 0, \"s\": 0, \"e\": 1, \"i\": {\"x\":[]}}]}}}]}";
+
+    SkMemoryStream stream(json, strlen(json));
+
+    // Passes if parsing doesn't crash.
+    auto animation = skottie::Animation::Make(&stream);
+}