GROOVY-12283: apply import rules to construction-coercion casts and subscripts

The indirect import check inspected constructor, method, static-method and
method-pointer expressions, so a class forbidden by the import rules could
still be built through a construction that is neither a constructor call nor
a method call: a cast whose operand is a list, map or closure literal
((Foo) [..], [..] as Foo, (Runnable) { }), and a named-argument subscript
(Foo[a: 1]). Each builds an instance of the named type.

The check is extended to both. A cast constructs when its operand is a list,
map or closure literal, as opposed to converting a value that already exists;
its target type is checked like a constructor call (array component unwrapped,
primitive components skipped as they name no class). A subscript constructs
when its arguments are map entries, which are not valid in an ordinary
subscript, so their presence marks the form unambiguously; the receiver type
is dynamic at this phase, so the class is named by its source text.

Plain converting casts ((String) x, (int) n) and positional subscripts stay
unexamined. The residual is the non-literal coercion ((Foo) var, var as Foo),
where an overridden asType could construct at runtime; that is statically
invisible and out of scope, consistent with this customizer being a hardening
aid rather than a security boundary.

Author: paulk-asert

src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java

@@ -19,6 +19,7 @@
 package org.codehaus.groovy.control.customizers;
 
 import org.codehaus.groovy.ast.ASTNode;
+import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.CodeVisitorSupport;
 import org.codehaus.groovy.ast.ConstructorNode;
@@ -850,9 +851,12 @@ public boolean isIndirectImportCheckEnabled() {
      * import statement, most usefully to prevent a class being instantiated by fully qualified name.
      * <p>
      * The rules are applied to the constructed type of a constructor call, to the receiver type of a
-     * method call or a static method call, and to the type a method pointer or method reference is
-     * taken on. They are not applied to every class node: class literals,
-     * property and attribute access, cast and declaration types, and catch types are not examined. Note
+     * method call or a static method call, to the type a method pointer or method reference is taken
+     * on, and to the constructed type of a construction by coercion &mdash; a cast whose operand is a
+     * list, map or closure literal ({@code (Foo) [..]}, {@code [..] as Foo}, {@code (Foo) { .. }})
+     * and a named-argument subscript ({@code Foo[a: 1]}), both of which build an instance of the
+     * named type. They are not applied to every class node: class literals, property and attribute
+     * access, plain (converting) cast and declaration types, and catch types are not examined. Note
      * also that the receiver type is the static type of that expression, which for a dynamically typed
      * receiver is {@code java.lang.Object} rather than the class the call reaches at runtime.
      *
@@ -1546,9 +1550,23 @@ protected void assertExpressionAuthorized(final Expression expression) throws Se
                         Expression methodName = expr.getMethodName();
                         assertStaticImportIsAllowed(methodName instanceof ConstantExpression
                                 ? methodName.getText() : null, typename);
+                    } else if (expression instanceof CastExpression expr && constructsByCoercion(expr)) {
+                        // GROOVY-12283: a cast whose operand is a list, map or closure literal
+                        // constructs an instance of the cast type (list/map -> constructor,
+                        // closure -> SAM proxy) rather than converting an existing value, so it
+                        // is checked like a constructor call. Covers `(Foo) [..]` and `[..] as Foo`.
+                        ClassNode target = getExpressionType(expr.getType()); // array -> component
+                        if (!ClassHelper.isPrimitiveType(target)) { // e.g. (int[]) [1, 2] has no class to check
+                            assertImportIsAllowed(target.getName());
+                        }
+                    } else if (expression instanceof BinaryExpression expr && isNamedArgConstruction(expr)) {
+                        // GROOVY-12283: `Foo[name: 'x', ..]` is a named-argument construction of
+                        // Foo, not a subscript (map entries are not valid in a real subscript). The
+                        // receiver type is dynamic here, so the class is named by its source text.
+                        assertImportIsAllowed(expr.getLeftExpression().getText());
                     }
                 } catch (SecurityException e) {
-                    throw new SecurityException("Indirect import checks prevents usage of expression", e);
+                    throw new SecurityException("Indirect import checks prevent usage of expression: " + e.getMessage(), e);
                 }
             }
         }
@@ -1563,6 +1581,49 @@ protected ClassNode getExpressionType(ClassNode objectExpressionType) {
             return objectExpressionType.isArray() ? getExpressionType(objectExpressionType.getComponentType()) : objectExpressionType;
         }
 
+        /**
+         * Whether a cast constructs an instance of its type by coercing a literal operand — a list
+         * or map (invoking a constructor) or a closure (creating a SAM proxy) — as opposed to
+         * converting a value that already exists. Such a cast is treated like a constructor call by
+         * the indirect import check (GROOVY-12283).
+         *
+         * @param cast the cast expression
+         * @return {@code true} if the cast materialises a new instance of its type
+         */
+        private static boolean constructsByCoercion(final CastExpression cast) {
+            Expression operand = cast.getExpression();
+            return operand instanceof ListExpression
+                    || operand instanceof MapExpression
+                    || operand instanceof ClosureExpression;
+        }
+
+        /**
+         * Whether a subscript is a named-argument construction such as
+         * {@code Foo[name: 'x', *: extra]} rather than an ordinary index access. Map entries are
+         * not valid in a real subscript, so their presence uniquely marks the construction form
+         * (GROOVY-12283).
+         * <p>
+         * A construction with only map entries or a bare spread ({@code Foo[a: 1]}, {@code Foo[*: m]})
+         * reaches the customizer already coerced to a {@link CastExpression} and is handled there;
+         * only a form mixing entries with a spread stays a subscript, so a {@link ListExpression}
+         * of arguments is the case to detect here.
+         *
+         * @param expression the binary expression
+         * @return {@code true} if the expression constructs by named arguments
+         */
+        private static boolean isNamedArgConstruction(final BinaryExpression expression) {
+            if (!"[".equals(expression.getOperation().getText())
+                    || !(expression.getRightExpression() instanceof ListExpression)) {
+                return false;
+            }
+            for (Expression element : ((ListExpression) expression.getRightExpression()).getExpressions()) {
+                if (element instanceof MapEntryExpression || element instanceof SpreadMapExpression) {
+                    return true;
+                }
+            }
+            return false;
+        }
+
         /**
          * Checks that a given token is either in the allowed list or not in the disallowed list.
          *

src/test/groovy/org/codehaus/groovy/control/customizers/SecureASTCustomizerTest.groovy

@@ -958,4 +958,80 @@ final class SecureASTCustomizerTest {
         '''
         // no error means success
     }
+
+    // GROOVY-12283: a cast whose operand is a list/map/closure literal, and a named-argument
+    // subscript, construct an instance of the named type rather than converting a value, so the
+    // indirect import check applies to them exactly as it does to a constructor call.
+
+    @Test
+    void testIndirectImportCheckBlocksCastAndAsListCoercion() {
+        customizer.allowedImports = ['java.lang.String']
+        customizer.indirectImportCheckEnabled = true
+        def shell = new GroovyShell(configuration)
+        // the constructor form is blocked already; the coercion forms build the same File
+        assert hasSecurityException { shell.evaluate("new java.io.File('/etc/passwd')") }
+        assert hasSecurityException { shell.evaluate("(java.io.File) ['/etc/passwd']") }
+        assert hasSecurityException { shell.evaluate("['/etc/passwd'] as java.io.File") }
+    }
+
+    @Test
+    void testIndirectImportCheckBlocksClosureCoercion() {
+        customizer.allowedImports = ['java.lang.String']
+        customizer.indirectImportCheckEnabled = true
+        def shell = new GroovyShell(configuration)
+        assert hasSecurityException { shell.evaluate("(Runnable) { }") }
+        assert hasSecurityException { shell.evaluate("{ -> } as Runnable") }
+    }
+
+    @Test
+    void testIndirectImportCheckBlocksNamedArgConstruction() {
+        customizer.disallowedImports = ['org.codehaus.groovy.control.customizers.SecGadget']
+        customizer.indirectImportCheckEnabled = true
+        def shell = new GroovyShell(configuration)
+        String g = 'org.codehaus.groovy.control.customizers.SecGadget'
+        // entry-only and explicit-cast forms coerce to a cast of a map literal (the cast branch)
+        assert hasSecurityException { shell.evaluate("${g}[a: 1]") }
+        assert hasSecurityException { shell.evaluate("(${g}) [a: 1]") }
+        // an entry mixed with a spread stays a subscript BinaryExpression (the subscript branch)
+        assert hasSecurityException { shell.evaluate("def m = [b: 2]; ${g}[a: 1, *: m]") }
+    }
+
+    @Test
+    void testIndirectImportCheckAllowsCoercionToPermittedType() {
+        customizer.allowedImports = ['java.io.File', 'java.lang.Runnable']
+        customizer.indirectImportCheckEnabled = true
+        def shell = new GroovyShell(configuration)
+        // the target types are permitted, so the coercions are permitted
+        shell.evaluate("(java.io.File) ['/tmp/x']")
+        shell.evaluate("['/tmp/x'] as java.io.File")
+        shell.evaluate("(Runnable) { }")
+    }
+
+    @Test
+    void testIndirectImportCheckLeavesInertCastsUnexamined() {
+        // a plain (converting) cast does not construct, so it is not checked even when its type
+        // is not on the allow list — this pins the slice boundary
+        customizer.allowedImports = ['java.util.ArrayList']
+        customizer.indirectImportCheckEnabled = true
+        def shell = new GroovyShell(configuration)
+        shell.evaluate("(CharSequence) 'hello'")     // operand is a value, not a literal coercion
+        shell.evaluate("def n = 1; (Number) n")
+        // and a genuine positional subscript is untouched
+        shell.evaluate("def list = [10, 20]; list[1]")
+        // a primitive-array coercion has no class name to check, so it is not blocked
+        shell.evaluate("(int[]) [1, 2, 3]")
+        shell.evaluate("[1, 2, 3] as int[]")
+        // a pure spread subscript coerces to `m as Foo` — a variable operand, the same
+        // non-literal coercion residual as `var as Foo`, so it is not examined (and constructs)
+        shell.evaluate("def m = [x: 1]; org.codehaus.groovy.control.customizers.SecGadget[*: m]")
+    }
+}
+
+/**
+ * Helper for {@link SecureASTCustomizerTest}: a class with a map constructor, referenced by
+ * fully qualified name so the indirect import check applies (GROOVY-12283).
+ */
+class SecGadget {
+    String tag
+    SecGadget(Map m) { tag = "map:$m" }
 }