commit 517f4ffb12ca5fae8ec812610691ae21525562e4
author Ethan Nicholas <ethannicholas@google.com> Thu Sep 02 10:57:45 2021 -0400
committer SkCQ <skcq-be@skia-corp.google.com.iam.gserviceaccount.com> Thu Sep 02 17:31:35 2021 +0000
tree de799514dd2e74942b1cec0e21c4b26a0a7e0efb
parent 79ff8f20bf26452fd5f0a1357861ebc103a86ab6

Added type security checks to the DSL

Private / unsupported types are now restricted to module code.

Change-Id: I98cb2e0822560a274758d99ecf0ca09883d0c3a1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445097
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

src/sksl/SkSLIRGenerator.cpp

diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 51ced68..f60ca6a 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -839,21 +839,6 @@
     }
 }
 
-static bool type_contains_private_fields(const Type& type) {
-    // Checks for usage of private types, including fields inside a struct.
-    if (type.isPrivate()) {
-        return true;
-    }
-    if (type.isStruct()) {
-        for (const auto& f : type.fields()) {
-            if (type_contains_private_fields(*f.fType)) {
-                return true;
-            }
-        }
-    }
-    return false;
-}
-
 const Type* IRGenerator::convertType(const ASTNode& type, bool allowVoid) {
     skstd::string_view name = type.getStringView();
     const Symbol* symbol = (*fSymbolTable)[name];
@@ -869,7 +854,7 @@
         return nullptr;
     }
     if (!fIsBuiltinCode) {
-        if (type_contains_private_fields(*result)) {
+        if (result->containsPrivateFields()) {
             this->errorReporter().error(type.fOffset, "type '" + name + "' is private");
             return nullptr;
         }

src/sksl/dsl/DSLType.cpp

diff --git a/src/sksl/dsl/DSLType.cpp b/src/sksl/dsl/DSLType.cpp
index 76fd7b1..2827f3a 100644
--- a/src/sksl/dsl/DSLType.cpp
+++ b/src/sksl/dsl/DSLType.cpp
@@ -27,7 +27,18 @@
                                               (int)name.length(), name.data()).c_str());
         return nullptr;
     }
-    return &symbol->as<Type>();
+    const Type& result = symbol->as<Type>();
+    if (!DSLWriter::IsModule()) {
+        if (result.containsPrivateFields()) {
+            DSLWriter::ReportError(("type '" + String(name) + "' is private").c_str());
+            return nullptr;
+        }
+        if (DSLWriter::Context().fConfig->strictES2Mode() && !result.allowedInES2()) {
+            DSLWriter::ReportError(("type '" + String(name) + "' is not supported").c_str());
+            return nullptr;
+        }
+    }
+    return &result;
 }
 
 static const Type* find_type(skstd::string_view name, const Modifiers& modifiers,

src/sksl/ir/SkSLType.cpp

diff --git a/src/sksl/ir/SkSLType.cpp b/src/sksl/ir/SkSLType.cpp
index 38db128..c30269a 100644
--- a/src/sksl/ir/SkSLType.cpp
+++ b/src/sksl/ir/SkSLType.cpp
@@ -749,6 +749,21 @@
     return this->isArray();
 }
 
+
+bool Type::containsPrivateFields() const {
+    if (this->isPrivate()) {
+        return true;
+    }
+    if (this->isStruct()) {
+        for (const auto& f : this->fields()) {
+            if (f.fType->containsPrivateFields()) {
+                return true;
+            }
+        }
+    }
+    return false;
+}
+
 bool Type::checkForOutOfRangeLiteral(const Context& context, const Expression& expr) const {
     bool foundError = false;
     const Type& baseType = this->componentType();

src/sksl/ir/SkSLType.h

diff --git a/src/sksl/ir/SkSLType.h b/src/sksl/ir/SkSLType.h
index a2f5a44..3023b82 100644
--- a/src/sksl/ir/SkSLType.h
+++ b/src/sksl/ir/SkSLType.h
@@ -511,6 +511,12 @@
     bool isOrContainsArray() const;
 
     /**
+     * Returns true if this type is either itself private or is a struct which contains private
+     * fields (recursively).
+     */
+    bool containsPrivateFields() const;
+
+    /**
      * Returns the corresponding vector or matrix type with the specified number of columns and
      * rows.
      */