From 94d4d3e20b8be29233bc7056ed3b8b36def3e98a Mon Sep 17 00:00:00 2001
From: Florin Malita <fmalita@chromium.org>
Date: Mon, 18 Jun 2018 13:10:51 -0400
Subject: [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>
---
 modules/skottie/BUILD.gn            | 20 ++++++++++++++++++++
 modules/skottie/src/SkottieJson.cpp |  8 ++++++--
 modules/skottie/src/SkottieTest.cpp | 23 +++++++++++++++++++++++
 3 files changed, 49 insertions(+), 2 deletions(-)
 create mode 100644 modules/skottie/src/SkottieTest.cpp

(limited to 'modules')

diff --git a/modules/skottie/BUILD.gn b/modules/skottie/BUILD.gn
index dff355cf82..9442592968 100644
--- a/modules/skottie/BUILD.gn
+++ b/modules/skottie/BUILD.gn
@@ -40,6 +40,26 @@ source_set("skottie") {
   }
 }
 
+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
diff --git a/modules/skottie/src/SkottieJson.cpp b/modules/skottie/src/SkottieJson.cpp
index 4f23939d15..186cd42c31 100644
--- a/modules/skottie/src/SkottieJson.cpp
+++ b/modules/skottie/src/SkottieJson.cpp
@@ -84,8 +84,12 @@ bool Parse<SkPoint>(const Value& v, SkPoint* pt) {
     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 <>
diff --git a/modules/skottie/src/SkottieTest.cpp b/modules/skottie/src/SkottieTest.cpp
new file mode 100644
index 0000000000..1c4cc078a5
--- /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);
+}
-- 
cgit v1.2.3