commit 0cc01b753ac6750e8decbcb92e4e33de1b147e20
author Florin Malita <fmalita@chromium.org> Thu May 10 18:40:35 2018 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Fri May 11 02:40:56 2018 +0000
tree 517b4f309268d2ffd53cea9cfc9005eff3cf926f
parent 03e7f36ee95e934c9d88af23530f7d4436896238

[skottie] Guard against asset cycles

Track assets being attached, and break cycles.

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

experimental/skottie/Skottie.cpp

diff --git a/experimental/skottie/Skottie.cpp b/experimental/skottie/Skottie.cpp
index 18c8c19..8396b5b 100644
--- a/experimental/skottie/Skottie.cpp
+++ b/experimental/skottie/Skottie.cpp
@@ -52,7 +52,12 @@
 
 namespace {
 
-using AssetMap = SkTHashMap<SkString, json::ValueRef>;
+struct AssetInfo {
+    json::ValueRef fAsset;
+    mutable bool   fIsAttaching; // Used for cycle detection
+};
+
+using AssetMap = SkTHashMap<SkString, AssetInfo>;
 
 struct AttachContext {
     const ResourceProvider& fResources;
@@ -756,16 +761,41 @@
     return sk_make_sp<SkottieSGAdapter>(std::move(animation));
 }
 
+sk_sp<sksg::RenderNode> AttachAssetRef(const json::ValueRef& jlayer, AttachContext* ctx,
+    sk_sp<sksg::RenderNode>(*attach_proc)(const json::ValueRef& comp, AttachContext* ctx)) {
+
+    const auto refId = jlayer["refId"].toDefault(SkString());
+    if (refId.isEmpty()) {
+        LOG("!! Layer missing refId\n");
+        return nullptr;
+    }
+
+    if (refId.startsWith("$")) {
+        return AttachNestedAnimation(refId.c_str() + 1, ctx);
+    }
+
+    const auto* asset_info = ctx->fAssets.find(refId);
+    if (!asset_info) {
+        LOG("!! Asset not found: '%s'\n", refId.c_str());
+        return nullptr;
+    }
+
+    if (asset_info->fIsAttaching) {
+        LOG("!! Asset cycle detected for: '%s'\n", refId.c_str());
+        return nullptr;
+    }
+
+    asset_info->fIsAttaching = true;
+    auto asset = attach_proc(asset_info->fAsset, ctx);
+    asset_info->fIsAttaching = false;
+
+    return asset;
+}
+
 sk_sp<sksg::RenderNode> AttachCompLayer(const json::ValueRef& jlayer, AttachContext* ctx,
                                         float* time_bias, float* time_scale) {
     SkASSERT(jlayer.isObject());
 
-    SkString refId;
-    if (!jlayer["refId"].to(&refId) || refId.isEmpty()) {
-        LOG("!! Comp layer missing refId\n");
-        return nullptr;
-    }
-
     const auto start_time = jlayer["st"].toDefault(0.0f),
              stretch_time = jlayer["sr"].toDefault(1.0f);
 
@@ -775,18 +805,7 @@
         *time_scale = 1;
     }
 
-    if (refId.startsWith("$")) {
-        return AttachNestedAnimation(refId.c_str() + 1, ctx);
-    }
-
-    const auto* comp = ctx->fAssets.find(refId);
-    if (!comp) {
-        LOG("!! Pre-comp not found: '%s'\n", refId.c_str());
-        return nullptr;
-    }
-
-    // TODO: cycle detection
-    return AttachComposition(*comp, ctx);
+    return AttachAssetRef(jlayer, ctx, AttachComposition);
 }
 
 sk_sp<sksg::RenderNode> AttachSolidLayer(const json::ValueRef& jlayer, AttachContext*,
@@ -831,23 +850,11 @@
         SkImage::MakeFromEncoded(SkData::MakeFromStream(resStream.get(), resStream->getLength())));
 }
 
-sk_sp<sksg::RenderNode> AttachImageLayer(const json::ValueRef& layer, AttachContext* ctx,
+sk_sp<sksg::RenderNode> AttachImageLayer(const json::ValueRef& jlayer, AttachContext* ctx,
                                          float*, float*) {
-    SkASSERT(layer.isObject());
+    SkASSERT(jlayer.isObject());
 
-    SkString refId;
-    if (!layer["refId"].to(&refId) || refId.isEmpty()) {
-        LOG("!! Image layer missing refId\n");
-        return nullptr;
-    }
-
-    const auto* jimage = ctx->fAssets.find(refId);
-    if (!jimage) {
-        LOG("!! Image asset not found: '%s'\n", refId.c_str());
-        return nullptr;
-    }
-
-    return AttachImageAsset(*jimage, ctx);
+    return AttachAssetRef(jlayer, ctx, AttachImageAsset);
 }
 
 sk_sp<sksg::RenderNode> AttachNullLayer(const json::ValueRef& layer, AttachContext*, float*, float*) {
@@ -1268,7 +1275,7 @@
     AssetMap assets;
     for (const json::ValueRef asset : json["assets"]) {
         if (asset.isObject()) {
-            assets.set(asset["id"].toDefault(SkString()), asset);
+            assets.set(asset["id"].toDefault(SkString()), { asset, false });
         }
     }