jenkinsci / groovy-sandbox

(Deprecated) Compile-time transformer to run Groovy code in a restrictive sandbox
MIT License
122 stars 60 forks source link

ArrayIndexOutOfBoundsException in GroovyCallSiteSelector.isIllegalCallToSyntheticConstructor #67

Closed haridsv closed 1 year ago

haridsv commented 3 years ago

I have script-security-plugin version 1.76 installed which depends on groovy-sandbox version 1.26. I started seeing the below exception in my pipeline after some recent updates and not sure exactly which update is triggering this error:

java.lang.ArrayIndexOutOfBoundsException: 1
    at org.kohsuke.groovy.sandbox.impl.GroovyCallSiteSelector.isIllegalCallToSyntheticConstructor(GroovyCallSiteSelector.java:124)
    at org.kohsuke.groovy.sandbox.impl.GroovyCallSiteSelector.findConstructor(GroovyCallSiteSelector.java:51)
    at org.kohsuke.groovy.sandbox.impl.Checker.checkedConstructor(Checker.java:201)
    at com.cloudbees.groovy.cps.sandbox.SandboxInvoker.constructorCall(SandboxInvoker.java:21)

I can reproduce this using this simple pipeline script with sandbox enabled:

public class Node {
    protected Node(String tagName, Node... children) {
    }
}

public static Node text(String text) {
    return new Node(text)
}

print(text("TEST"))

Full exception stack trace:

java.lang.ArrayIndexOutOfBoundsException: 1
    at org.kohsuke.groovy.sandbox.impl.GroovyCallSiteSelector.isIllegalCallToSyntheticConstructor(GroovyCallSiteSelector.java:124)
    at org.kohsuke.groovy.sandbox.impl.GroovyCallSiteSelector.findConstructor(GroovyCallSiteSelector.java:51)
    at org.kohsuke.groovy.sandbox.impl.Checker.checkedConstructor(Checker.java:201)
    at com.cloudbees.groovy.cps.sandbox.SandboxInvoker.constructorCall(SandboxInvoker.java:21)
    at WorkflowScript.text(WorkflowScript:9)
    at WorkflowScript.run(WorkflowScript:12)
    at ___cps.transform___(Native Method)
    at com.cloudbees.groovy.cps.impl.FunctionCallBlock$ContinuationImpl.dispatchOrArg(FunctionCallBlock.java:97)
    at com.cloudbees.groovy.cps.impl.FunctionCallBlock$ContinuationImpl.fixArg(FunctionCallBlock.java:83)
    at sun.reflect.GeneratedMethodAccessor222.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at com.cloudbees.groovy.cps.impl.ContinuationPtr$ContinuationImpl.receive(ContinuationPtr.java:72)
    at com.cloudbees.groovy.cps.impl.LocalVariableBlock$LocalVariable.get(LocalVariableBlock.java:39)
    at com.cloudbees.groovy.cps.LValueBlock$GetAdapter.receive(LValueBlock.java:30)
    at com.cloudbees.groovy.cps.impl.LocalVariableBlock.evalLValue(LocalVariableBlock.java:28)
    at com.cloudbees.groovy.cps.LValueBlock$BlockImpl.eval(LValueBlock.java:55)
    at com.cloudbees.groovy.cps.LValueBlock.eval(LValueBlock.java:16)
    at com.cloudbees.groovy.cps.Next.step(Next.java:83)
    at com.cloudbees.groovy.cps.Continuable$1.call(Continuable.java:174)
    at com.cloudbees.groovy.cps.Continuable$1.call(Continuable.java:163)
    at org.codehaus.groovy.runtime.GroovyCategorySupport$ThreadCategoryInfo.use(GroovyCategorySupport.java:129)
    at org.codehaus.groovy.runtime.GroovyCategorySupport.use(GroovyCategorySupport.java:268)
    at com.cloudbees.groovy.cps.Continuable.run0(Continuable.java:163)
    at org.jenkinsci.plugins.workflow.cps.SandboxContinuable.access$001(SandboxContinuable.java:18)
    at org.jenkinsci.plugins.workflow.cps.SandboxContinuable.run0(SandboxContinuable.java:51)
    at org.jenkinsci.plugins.workflow.cps.CpsThread.runNextChunk(CpsThread.java:185)
    at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup.run(CpsThreadGroup.java:400)
    at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup.access$400(CpsThreadGroup.java:96)
    at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup$2.call(CpsThreadGroup.java:312)
    at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup$2.call(CpsThreadGroup.java:276)
    at org.jenkinsci.plugins.workflow.cps.CpsVmExecutorService$2.call(CpsVmExecutorService.java:67)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at hudson.remoting.SingleLaneExecutorService$1.run(SingleLaneExecutorService.java:131)
    at jenkins.util.ContextResettingExecutorService$1.run(ContextResettingExecutorService.java:28)
    at jenkins.security.ImpersonatingExecutorService$1.run(ImpersonatingExecutorService.java:59)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)

If I remove the variable number of arguments from Node, it works fine.

We are using Jenkins 2.176.4.

haridsv commented 3 years ago

The below variant works fine:

public class Node {
    protected Node(String tagName, Node[] children=null) {
    }
}

public static Node text(String text) {
    return new Node(text)
}

print(text("TEST"))
dwnusbaum commented 3 years ago

Thanks for reporting this! It looks like any use of varargs in constructors will have this issue unless you pass exactly one argument for the varargs parameter. I think it could be fixed relatively easy by changing the logic when Constructor.isVarArgs returns true in GroovyCallSiteSelector.isIllegalCallToSyntheticConstructor. Thankfully there is a pretty straightforward workaround: use explicit arrays or lists instead of varargs.

haridsv commented 3 years ago

Thankfully there is a pretty straightforward workaround: use explicit arrays or lists instead of varargs.

Right, very simple workaround. Thankfully, I already have factory methods that take varargs so I could change the signature of the constructor without impacting any of the client code.

dwnusbaum commented 1 year ago

This was fixed incidentally as part of https://github.com/jenkinsci/groovy-sandbox/commit/520243213bcd8c81322e8e683daa8d555bb4f484 and was released in version 1.30.