Code Monkey home page Code Monkey logo

Comments (5)

robinfriedli avatar robinfriedli commented on August 12, 2024

My current workaround is to do the following, which also seems to work

def noneMatch = list.stream().noneMatch({s -> s.indexOf((int) somec) >= 0})
while (noneMatch) {
    somec++
    noneMatch = list.stream().noneMatch({s -> s.indexOf((int) somec) >= 0})
}

from groovy-sandbox.

dwnusbaum avatar dwnusbaum commented on August 12, 2024

Thanks for the bug report! I looked into this today to try to understand whether the bug could be used to bypass the sandbox by confusing the transformer. I think it's safe, since the problem appears to be that the expression in question is being transformed by the sandbox multiple times.

This happens because ClassCodeExpressionTransformer.visitWhileLoop first transforms the condition expression for the loop, then visits the body by calling super.visitWhileLoop, which ends up visiting the condition expression again. Because of this, the same problem probably also affects do-while loops, and maybe also for loops, since the visitor code for them uses similar logic.

Explanation of the bug and the patch I came up with while testing:

When ClassCodeExpressionTransformer.visitWhileLoop is called, everything works fine in the call to transform the condition expression. Inside of SandboxTransformer$VisitorImpl.innerTransform, the parameter for the closure is declared correctly, and so when the variable is transformed it gets ignored here because it is a local variable.

The call to super.visitWhileLoop is where things go wrong. That method ends up calling CodeVisitorSupport.visitWhileLoop, which visits the condition expression again, but through visit instead of transform. The condition expression does not go through transform until visitExpressionStatement is called on the body of the closure, which means that the parameters of the closure are not declared in the current scope, and so we go through the else branch here when transforming s, which ends up causing the problem.

I think the fix is relatively straightforward. ScopeTrackingClassCodeExpressionTransformer.visitClosureExpression needs to declare the closure parameters before visiting the body so that they are in scope when s is transformed for the second time. Similar things are being done there for other AST elements that can declare new parameters, such as in visitMethod, visitForLoop, and visitCatchStatement.

diff --git a/src/main/java/org/kohsuke/groovy/sandbox/ScopeTrackingClassCodeExpressionTransformer.java b/src/main/java/org/kohsuke/groovy/sandbox/ScopeTrackingClassCodeExpressionTransformer.java
index 8893207..59d8b81 100644
--- a/src/main/java/org/kohsuke/groovy/sandbox/ScopeTrackingClassCodeExpressionTransformer.java
+++ b/src/main/java/org/kohsuke/groovy/sandbox/ScopeTrackingClassCodeExpressionTransformer.java
@@ -1,6 +1,7 @@
 package org.kohsuke.groovy.sandbox;
 
 import org.codehaus.groovy.ast.ClassCodeExpressionTransformer;
+import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.FieldNode;
 import org.codehaus.groovy.ast.MethodNode;
 import org.codehaus.groovy.ast.Parameter;
@@ -154,6 +155,14 @@ abstract class ScopeTrackingClassCodeExpressionTransformer extends ClassCodeExpr
     @Override
     public void visitClosureExpression(ClosureExpression expression) {
         try (StackVariableSet scope = new StackVariableSet(this)) {
+            Parameter[] parameters = expression.getParameters();
+            if (parameters != null && parameters.length > 0) {
+                for (Parameter p : parameters) {
+                    declareVariable(p);
+                }
+            } else {
+                declareVariable(new Parameter(ClassHelper.DYNAMIC_TYPE, "it"));
+            }
             super.visitClosureExpression(expression);
         }
     }
diff --git a/src/test/java/org/kohsuke/groovy/sandbox/SandboxTransformerTest.java b/src/test/java/org/kohsuke/groovy/sandbox/SandboxTransformerTest.java
index 9cf1de8..5f030da 100644
--- a/src/test/java/org/kohsuke/groovy/sandbox/SandboxTransformerTest.java
+++ b/src/test/java/org/kohsuke/groovy/sandbox/SandboxTransformerTest.java
@@ -336,4 +336,21 @@ public class SandboxTransformerTest {
                 "System:getProperties()");
     }
 
+    @Test public void closureVariables() throws Exception {
+        isolate(() -> assertIntercept(
+                "while ([false].stream().noneMatch({s -> s})) {\n" +
+                "    return true\n" +
+                "}\n" +
+                "return false\n",
+                true,
+                "ArrayList.stream()", "ReferencePipeline$Head.noneMatch(Script1$_run_closure1)"));
+        isolate(() -> assertIntercept(
+                "while ([false].stream().noneMatch({it})) {\n" +
+                "    return true\n" +
+                "}\n" +
+                "return false\n",
+                true,
+                "ArrayList.stream()", "ReferencePipeline$Head.noneMatch(Script2$_run_closure1)"));
+    }
+
 }

from groovy-sandbox.

robinfriedli avatar robinfriedli commented on August 12, 2024

Has this been forgotten? Or waiting for approval?

from groovy-sandbox.

ashenp avatar ashenp commented on August 12, 2024

Hi:
Is any realeased version which has solved this bug?

from groovy-sandbox.

GeoPakas avatar GeoPakas commented on August 12, 2024

Any resolution for this bug nowadays?

from groovy-sandbox.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.