jenkinsci / groovy-sandbox

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

Static type checking error in sandbox-transformed code #117

Open SaschaRiemer opened 7 months ago

SaschaRiemer commented 7 months ago

Jenkins and plugins versions report

Environment ```text Jenkins: 2.414.3 OS: Linux - 5.14.21-150500.55.31-default Java: 11.0.20.1 - Eclipse Adoptium (OpenJDK 64-Bit Server VM) --- Office-365-Connector:4.20.0 PrioritySorter:5.0.0 SquishPlugin:8.6 active-directory:2.34 analysis-model-api:11.13.0 ant:497.v94e7d9fffa_b_9 antisamy-markup-formatter:162.v0e6ec0fcfcf6 apache-httpcomponents-client-4-api:4.5.14-208.v438351942757 apache-httpcomponents-client-5-api:5.2.1-1.1 artifactory:4.0.0 authentication-tokens:1.53.v1c90fd9191a_b_ authorize-project:1.7.1 azure-ad:433.v1982e2b_b_4a_fe azure-sdk:157.v855da_0b_eb_dc2 blueocean:1.27.9 blueocean-autofavorite:1.2.5 blueocean-bitbucket-pipeline:1.27.9 blueocean-commons:1.27.9 blueocean-config:1.27.9 blueocean-core-js:1.27.9 blueocean-dashboard:1.27.9 blueocean-display-url:2.4.2 blueocean-events:1.27.9 blueocean-git-pipeline:1.27.9 blueocean-github-pipeline:1.27.9 blueocean-i18n:1.27.9 blueocean-jira:1.27.9 blueocean-jwt:1.27.9 blueocean-personalization:1.27.9 blueocean-pipeline-api-impl:1.27.9 blueocean-pipeline-editor:1.27.9 blueocean-pipeline-scm-api:1.27.9 blueocean-rest:1.27.9 blueocean-rest-impl:1.27.9 blueocean-web:1.27.9 bootstrap5-api:5.3.2-2 bouncycastle-api:2.29 branch-api:2.1128.v717130d4f816 caffeine-api:3.1.8-133.v17b_1ff2e0599 checks-api:2.0.2 cloud-stats:320.v96b_65297a_4b_b_ cloudbees-bitbucket-branch-source:848.v42c6a_317eda_e cloudbees-folder:6.858.v898218f3609d command-launcher:107.v773860566e2e commons-httpclient3-api:3.1-3 commons-lang3-api:3.13.0-62.v7d18e55f51e2 commons-text-api:1.11.0-94.v3e1f4a_926e49 config-file-provider:959.vcff671a_4518b_ configuration-as-code:1737.v652ee9b_a_e0d9 copyartifact:722.v0662a_9b_e22a_c cors-filter:1.1 credentials:1309.v8835d63eb_d8a_ credentials-binding:642.v737c34dea_6c2 csp:1.2 custom-tools-plugin:0.8 data-tables-api:1.13.6-5 disable-github-multibranch-status:1.2 display-url-api:2.200.vb_9327d658781 docker-commons:439.va_3cb_0a_6a_fb_29 docker-java-api:3.3.1-79.v20b_53427e041 docker-plugin:1.5 docker-workflow:572.v950f58993843 dtkit-api:3.0.2 durable-task:523.va_a_22cf15d5e0 echarts-api:5.4.0-7 email-ext:2.102 extended-choice-parameter:376.v2e02857547b_a_ external-workspace-manager:1.3.1 favorite:2.4.3 file-operations:177.vd1773063d935 font-awesome-api:6.4.2-1 forensics-api:2.3.0 generic-webhook-trigger:1.88.0 git:5.2.1 git-client:4.5.0 github:1.37.3.1 github-api:1.316-451.v15738eef3414 github-branch-source:1741.va_3028eb_9fd21 github-checks:554.vb_ee03a_000f65 gradle:2.9 handy-uri-templates-2-api:2.1.8-22.v77d5b_75e6953 html5-notifier-plugin:1.5 htmlpublisher:1.32 http_request:1.18 instance-identity:185.v303dc7c645f9 ionicons-api:56.v1b_1c8c49374e jackson2-api:2.15.3-372.v309620682326 jacoco:3.3.5 jakarta-activation-api:2.0.1-3 jakarta-mail-api:2.0.1-3 javadoc:243.vb_b_503b_b_45537 javax-activation-api:1.2.0-6 javax-mail-api:1.6.2-9 jaxb:2.3.9-1 jdk-tool:73.vddf737284550 jenkins-design-language:1.27.9 jersey2-api:2.41-133.va_03323b_a_1396 jira:3.11 jira-steps:2.0.165.v8846cf59f3db jjwt-api:0.11.5-77.v646c772fddb_0 job-dsl:1.87 job-restrictions:0.8 jquery3-api:3.7.1-1 jsch:0.2.8-65.v052c39de79b_2 junit:1240.vf9529b_881428 ldap:711.vb_d1a_491714dc locale:314.v22ce953dfe9e lockable-resources:1185.v0c528656ce04 mailer:463.vedf8358e006b_ mapdb-api:1.0.9-28.vf251ce40855d matrix-auth:3.2.1 matrix-project:818.v7eb_e657db_924 maven-plugin:3.23 metrics:4.2.18-442.v02e107157925 mina-sshd-api-common:2.11.0-86.v836f585d47fa_ mina-sshd-api-core:2.11.0-86.v836f585d47fa_ monitoring:1.95.0 oic-auth:2.6 okhttp-api:4.11.0-157.v6852a_a_fa_ec11 pam-auth:1.10 parameterized-scheduler:255.v73827fcdf618 parameterized-trigger:2.46 pipeline-build-step:516.v8ee60a_81c5b_9 pipeline-graph-analysis:202.va_d268e64deb_3 pipeline-groovy-lib:689.veec561a_dee13 pipeline-input-step:477.v339683a_8d55e pipeline-milestone-step:111.v449306f708b_7 pipeline-model-api:2.2150.v4cfd8916915c pipeline-model-definition:2.2150.v4cfd8916915c pipeline-model-extensions:2.2150.v4cfd8916915c pipeline-rest-api:2.34 pipeline-stage-step:305.ve96d0205c1c6 pipeline-stage-tags-metadata:2.2150.v4cfd8916915c pipeline-stage-view:2.34 pipeline-utility-steps:2.16.0 plain-credentials:143.v1b_df8b_d3b_e48 plugin-util-api:3.6.0 prism-api:1.29.0-8 progress-bar-column-plugin:11.vdef198c2d6c1 prometheus:2.3.3 pubsub-light:1.18 schedule-build:502.v9379e178e65b_ scm-api:676.v886669a_199a_a_ script-security:1275.v23895f409fb_d shelve-project-plugin:3.2 snakeyaml-api:2.2-111.vc6598e30cc65 sse-gateway:1.26 ssh-credentials:308.ve4497b_ccd8f4 ssh-slaves:2.916.vd17b_43357ce4 sshd:3.312.v1c601b_c83b_0e structs:325.vcb_307d2a_2782 subversion:2.17.3 throttle-concurrents:2.14 timestamper:1.26 tm4j-automation:3.3.4 token-macro:384.vf35b_f26814ec trilead-api:2.84.v72119de229b_7 variant:60.v7290fc0eb_b_cd warnings-ng:10.5.1 workflow-aggregator:596.v8c21c963d92d workflow-api:1283.v99c10937efcb_ workflow-basic-steps:1042.ve7b_140c4a_e0c workflow-cps:3806.va_3a_6988277b_2 workflow-durable-task-step:1289.v4d3e7b_01546b_ workflow-job:1360.vc6700e3136f5 workflow-multibranch:756.v891d88f2cd46 workflow-scm-step:415.v434365564324 workflow-step-api:639.v6eca_cd8c04a_a_ workflow-support:865.v43e78cc44e0d xunit:3.1.3 zentimestamp:4.2 ```

What Operating System are you using (both controller, and any agents involved in the problem)?

Windows and Linux both for the controller. An agent is not needed to reproduce the issue.

Reproduction steps

  1. (Try to) execute this pipeline:
    
    import groovy.transform.CompileStatic

@CompileStatic class Test { @NonCPS static int foo() { return Integer.valueOf("5") } }

pipeline { agent none stages { stage('Test') { steps { echo "${Test.foo()}" } } } }


### Expected Results

The pipeline should complete successfully

### Actual Results

The following error is written in the console log:

org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed: WorkflowScript: 7: [Static type checking] - Non static method java.util.List#toArray cannot be called from static context @ line 7, column 32. return Integer.valueOf("5") ^

1 error

at org.codehaus.groovy.control.ErrorCollector.failIfErrors(ErrorCollector.java:309)
at org.codehaus.groovy.control.CompilationUnit.applyToPrimaryClassNodes(CompilationUnit.java:1107)
at org.codehaus.groovy.control.CompilationUnit.doPhaseOperation(CompilationUnit.java:624)
at org.codehaus.groovy.control.CompilationUnit.processPhaseOperations(CompilationUnit.java:602)
at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:579)
at groovy.lang.GroovyClassLoader.doParseClass(GroovyClassLoader.java:323)
at groovy.lang.GroovyClassLoader.parseClass(GroovyClassLoader.java:293)
at org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.GroovySandbox$Scope.parse(GroovySandbox.java:163)
at org.jenkinsci.plugins.workflow.cps.CpsGroovyShell.doParse(CpsGroovyShell.java:190)
at org.jenkinsci.plugins.workflow.cps.CpsGroovyShell.reparse(CpsGroovyShell.java:175)
at org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.parseScript(CpsFlowExecution.java:635)
at org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.start(CpsFlowExecution.java:581)
at org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:335)
at hudson.model.ResourceController.execute(ResourceController.java:101)
at hudson.model.Executor.run(Executor.java:442)

### Anything else?

The sandbox transformer will create code for the method foo, which should look like this:
static int foo() {
    return Checker.checkedCall(Integer, false, false, "valueOf", ["5"].toArray())
}

Here you can actually see the toArray call, which the static type checking complains about.

When directly writing this code and inspecting the generated groovy AST, ["5"].toArray() is a MethodCallExpression, with a ListExpression as object, "toArray" as the method, and empty argument list.
The difference between the parsed groovy code and the one created by the SandboxTransformer is the attribute implicitThis on the MethodCallExpression.
* In the parsed groovy code it is false (presumably, because the receiver object, the list expression, is explicitly given)
* In the SandboxTransformer code, it is true (because that is the default value)

### Are you interested in contributing a fix?

Not tested yet, but I'm assuming that changing the created MethodCallExpression in SandboxTransformer.transformArguments should fix this:
        MethodCallExpression mce = new MethodCallExpression(new ListExpression(l),"toArray",new ArgumentListExpression());
        mce.setImplicitThis(false);
        return withLoc(e, mce);
dwnusbaum commented 6 months ago

Thanks for the investigation! I checked your proposed change and indeed that does seem to fix this specific issue. From a very brief look I could not reproduce a similar issue with the other uses of new MethodCallExpression in this code, which was a bit interesting.

Note though that I think this is probably just the tip of the iceberg as far as @CompileStatic goes, see for example https://github.com/jenkinsci/groovy-sandbox/issues/20. In general I would consider it unsupported. (In hindsight though, it probably would have been better to make groovy-sandbox and groovy-cps forcibly apply @CompileStatic to all explicitly declared classes in scripts, which I think would have prevented a lot of the security issues we've had to fix that are related to Groovy's dynamic behavior.)