Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-50380

script-security doesn't handle a non-standard Collection without a constructor taking an array properly

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      Pipeline job fails after upgrading Jenkins from 2.32.1.1 to 2.89.4.2. The following error is returned although Script approval list is empty:

      org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: unclassified new common.EnvsToBuild common.Env
       at org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SandboxInterceptor.onNewInstance(SandboxInterceptor.java:144)
       at org.kohsuke.groovy.sandbox.impl.Checker$14.call(Checker.java:580)
       at org.kohsuke.groovy.sandbox.impl.Checker.checkedCast(Checker.java:585)
       at org.kohsuke.groovy.sandbox.impl.Checker$checkedCast.callStatic(Unknown Source)
       at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallStatic(CallSiteArray.java:56)
       at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callStatic(AbstractCallSite.java:194)
       at org.kohsuke.groovy.sandbox.impl.Checker$2.call(Checker.java:188)
       at org.kohsuke.groovy.sandbox.GroovyInterceptor.onStaticCall(GroovyInterceptor.java:35)
       at org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SandboxInterceptor.onStaticCall(SandboxInterceptor.java:158)
       at org.kohsuke.groovy.sandbox.impl.Checker$2.call(Checker.java:186)
       at org.kohsuke.groovy.sandbox.impl.Checker.checkedStaticCall(Checker.java:190)
       at org.kohsuke.groovy.sandbox.impl.Checker.checkedCall(Checker.java:97)
       at com.cloudbees.groovy.cps.sandbox.SandboxInvoker.methodCall(SandboxInvoker.java:17)
       at WorkflowScript.run(WorkflowScript:5)
       at __cps.transform__(Native Method)
       at com.cloudbees.groovy.cps.impl.ContinuationGroup.methodCall(ContinuationGroup.java:57)
       at com.cloudbees.groovy.cps.impl.FunctionCallBlock$ContinuationImpl.dispatchOrArg(FunctionCallBlock.java:109)
       at com.cloudbees.groovy.cps.impl.FunctionCallBlock$ContinuationImpl.fixArg(FunctionCallBlock.java:82)
       at sun.reflect.GeneratedMethodAccessor224.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.ConstantBlock.eval(ConstantBlock.java:21)
       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:122)
       at org.codehaus.groovy.runtime.GroovyCategorySupport.use(GroovyCategorySupport.java:261)
       at com.cloudbees.groovy.cps.Continuable.run0(Continuable.java:163)
       at org.jenkinsci.plugins.workflow.cps.SandboxContinuable.access$001(SandboxContinuable.java:19)
       at org.jenkinsci.plugins.workflow.cps.SandboxContinuable$1.call(SandboxContinuable.java:35)
       at org.jenkinsci.plugins.workflow.cps.SandboxContinuable$1.call(SandboxContinuable.java:32)
       at org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.GroovySandbox.runInSandbox(GroovySandbox.java:108)
       at org.jenkinsci.plugins.workflow.cps.SandboxContinuable.run0(SandboxContinuable.java:32)
       at org.jenkinsci.plugins.workflow.cps.CpsThread.runNextChunk(CpsThread.java:174)
       at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup.run(CpsThreadGroup.java:331)
       at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup.access$200(CpsThreadGroup.java:82)
       at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup$2.call(CpsThreadGroup.java:243)
       at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup$2.call(CpsThreadGroup.java:231)
       at org.jenkinsci.plugins.workflow.cps.CpsVmExecutorService$2.call(CpsVmExecutorService.java:64)
       at java.util.concurrent.FutureTask.run(FutureTask.java:266)
       at hudson.remoting.SingleLaneExecutorService$1.run(SingleLaneExecutorService.java:112)
       at jenkins.util.ContextResettingExecutorService$1.run(ContextResettingExecutorService.java:28)
       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:1142)
       at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
       at java.lang.Thread.run(Thread.java:745)
       Finished: FAILURE

      Pipeline code that's not working:

      import common.EnvsToBuild
      @Library('jenkins-groovy-error-reproduction-library') _
      EnvsToBuild envsToBuild = new EnvsToBuild(true, false) 
      println envsToBuild
      //println "${new EnvsToBuild(true, false)}"

      Workaround:

      import common.EnvsToBuild
      @Library('jenkins-groovy-error-reproduction-library') _
      //EnvsToBuild envsToBuild = new EnvsToBuild(true, false) 
      //println envsToBuild
      println "${new EnvsToBuild(true, false)}"
      
      import common.EnvsToBuild
      @Library('jenkins-groovy-error-reproduction-library') _
      EnvsToBuild envsToBuild;
      envsToBuild = new EnvsToBuild(true, false) 
      println envsToBuild
      
      import common.EnvsToBuild
      @Library('jenkins-groovy-error-reproduction-library') _
      def envsToBuild = new EnvsToBuild(true, false) 
      println envsToBuild
      

       

        Attachments

          Issue Links

            Activity

            Hide
            abayer Andrew Bayer added a comment -

            So: this was introduced in script-security 1.31 as part of one of a number of security fixes relating to casting. There are actually two things going on here - first is that the logic for actually doing a sandbox-checked cast still tries to do a cast even when the target class and the class of the object to cast are exactly the same. That's obviously goofy and shouldn't happen. The second is that the logic for doing the sandbox-checked cast sees any Collection that isn't in java.util (those are handled natively by Groovy so we don't worry about them) and, rather than simply casting it, it does a new ClassInQuestion(original.toArray()). Which normally works for a Collection since most will have a constructor taking an array of objects, but doesn't work in this case, where there is no such constructor.

            While I do believe the cast-to-same-class case should be fixed in general, and I am not pleased with the array constructor for a Collection thing, the bigger issue here is that I don't think we should be ending up in a sandbox-checked cast in this situation in the first place. However, I can't figure out how to prevent that without reopening the casting related issues this is dealing with in the first place...so it may be that we need to accept the potentially pointless sandbox-checked cast, but ensure that said sandbox-checked cast will check at runtime if a straight Class#cast call would suffice and do that instead of handing off to Groovy's casting magic...

            Show
            abayer Andrew Bayer added a comment - So: this was introduced in script-security 1.31 as part of one of a number of security fixes relating to casting. There are actually two things going on here - first is that the logic for actually doing a sandbox-checked cast still tries to do a cast even when the target class and the class of the object to cast are exactly the same. That's obviously goofy and shouldn't happen. The second is that the logic for doing the sandbox-checked cast sees any Collection that isn't in java.util (those are handled natively by Groovy so we don't worry about them) and, rather than simply casting it, it does a new ClassInQuestion(original.toArray()) . Which normally works for a Collection since most will have a constructor taking an array of objects, but doesn't work in this case, where there is no such constructor. While I do believe the cast-to-same-class case should be fixed in general, and I am not pleased with the array constructor for a Collection thing, the bigger issue here is that I don't think we should be ending up in a sandbox-checked cast in this situation in the first place. However, I can't figure out how to prevent that without reopening the casting related issues this is dealing with in the first place...so it may be that we need to accept the potentially pointless sandbox-checked cast, but ensure that said sandbox-checked cast will check at runtime if a straight Class#cast call would suffice and do that instead of handing off to Groovy's casting magic...
            Hide
            abayer Andrew Bayer added a comment -

            Ok, I've got PRs up for groovy-sandbox and script-security that fix this by making sure Checker.checkedCast doesn't bother jumping through the DefaultTypeTransformation.castToType hoops if a simple clazz.cast will do the trick. Let's see what the reviewers think.

            Show
            abayer Andrew Bayer added a comment - Ok, I've got PRs up for groovy-sandbox and script-security that fix this by making sure Checker.checkedCast doesn't bother jumping through the DefaultTypeTransformation.castToType hoops if a simple clazz.cast will do the trick. Let's see what the reviewers think.
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Andrew Bayer
            Path:
            pom.xml
            src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java
            http://jenkins-ci.org/commit/script-security-plugin/c307bcc62f6170ec6e3f1a71b986c723567f42e1
            Log:
            JENKINS-50380 checkedCast should use clazz.cast when assignable

            Downstream of https://github.com/jenkinsci/groovy-sandbox/pull/45

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Andrew Bayer Path: pom.xml src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java http://jenkins-ci.org/commit/script-security-plugin/c307bcc62f6170ec6e3f1a71b986c723567f42e1 Log: JENKINS-50380 checkedCast should use clazz.cast when assignable Downstream of https://github.com/jenkinsci/groovy-sandbox/pull/45
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Andrew Bayer
            Path:
            pom.xml
            src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java
            http://jenkins-ci.org/commit/script-security-plugin/c9ecc7957e742b042326173e314a9e6619a8def4
            Log:
            Merge pull request #192 from abayer/jenkins-50380

            JENKINS-50380 checkedCast should just return object when assignable

            Compare: https://github.com/jenkinsci/script-security-plugin/compare/2fa618d6f534...c9ecc7957e74

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Andrew Bayer Path: pom.xml src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java http://jenkins-ci.org/commit/script-security-plugin/c9ecc7957e742b042326173e314a9e6619a8def4 Log: Merge pull request #192 from abayer/jenkins-50380 JENKINS-50380 checkedCast should just return object when assignable Compare: https://github.com/jenkinsci/script-security-plugin/compare/2fa618d6f534...c9ecc7957e74
            Hide
            abayer Andrew Bayer added a comment -

            Merged, releasing momentarily as part of script-security 1.43.

            Show
            abayer Andrew Bayer added a comment - Merged, releasing momentarily as part of script-security 1.43.

              People

              • Assignee:
                abayer Andrew Bayer
                Reporter:
                rzhou Ronnie Zhou
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: