-
Bug
-
Resolution: Unresolved
-
Major
-
None
-
Jenkins 2.479.1
git 5.7.0
script-security 1369.v9b_98a_4e95b_2d
workflow-multibranch 800.v5f0a_a_660950e
Jenkins is throwing a RejectedAccessException despise the GitSCM.getBrowser() method being whitelisted. SCM.getBrowser() is not whitelisted.
When multiple classes define / overload a method the script-security plugin selects the original declaring class instead of the overloading child class.
Given the following Jenkinsfile multi-branch pipeline backed by git:
pipeline {
agent any
stages {
stage('Stage') {
steps {
script {
println "class: " + scm.class
println "browser: " + scm.browser
}
}
}
}
}
The error:
13:22:25 [Pipeline] echo 13:22:25 class: class hudson.plugins.git.GitSCM 13:22:25 Scripts not permitted to use method hudson.scm.SCM getBrowser. Administrators can decide whether to approve or reject this signature. 13:22:25 [Pipeline] } . . . 13:22:25 org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use method hudson.scm.SCM getBrowser 13:22:25 at PluginClassLoader for script-security//org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.StaticWhitelist.rejectMethod(StaticWhitelist.java:244) 13:22:25 at PluginClassLoader for script-security//org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SandboxInterceptor.rejectMethod(SandboxInterceptor.java:594) 13:22:25 at PluginClassLoader for script-security//org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SandboxInterceptor.lambda$onGetProperty$7(SandboxInterceptor.java:302) 13:22:25 at PluginClassLoader for script-security//org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SandboxInterceptor.onGetProperty(SandboxInterceptor.java:386)
See this comment for further analysis.
- relates to
-
JENKINS-24982 Bottom-up white/blacklisting vs top-down
-
- Open
-
[JENKINS-75288] scm.browser RejectedAccessException despite method being whitelisted
The GroovyCallSiteSelector.method method is used to determine the Method object being acted upon. It does consider the type hierarchy but the type hierarchy is ordered by supertypes first. According to this comment it is intentional. As a result all security checking is done on the original method declaring class instead of the class overriding the method.
For the bug in this ticket:
receiver = GitSCM@18657 method = "getBrowser" args = {Object[0]@18659} types = {LinkedHashSet@18806} size = 7 0 = {Class@613} "class java.lang.Object" 1 = {Class@6567} "interface hudson.model.Describable" 2 = {Class@6569} "interface hudson.ExtensionPoint" 3 = {Class@8713} "class hudson.scm.SCM" 4 = {Class@610} "interface java.io.Serializable" 5 = {Class@8760} "class hudson.plugins.git.GitSCMBackwardCompatibility" 6 = {Class@8835} "class hudson.plugins.git.GitSCM"
There are two classes that declare the method:
- method hudson.scm.SCM getBrowser
- method hudson.scm.GitSCM getBrowser // This is whitelisted by an annotation. GitSCM overrides SCM.getBrowser.
Since parent types are checked first it always picks method hudson.scm.SCM getBrowser.
Honestly this seems backwards to me. Whitelisting should default to least privileged and thus should check the most refined child declaring class instead of the most broad parent class. Also, just because I decided a method is okay to use doesn't mean any child overridden implementations are okay. With the current implementation it's not possible to whitelist an overridden implementation of method (or field).
It looks like it has been this way since the initial version (there was a refactor in-between). Based on the commit comment perhaps a copy-paste from the cloudbees-template plugin? I don't have access to that source code so can't trace this any further.
I think the fix for this would be to just change the ordering to check subclasses first? It wouldn't be without impact though. Any whitelists in this situation (method overloading) may need to be updated. It would also mean whitelisting would become more tedious (for example you could no longer blanket whitelist toString() - custom overrides would need to be explicitly whitelisted)
Poking jglick for comment 
Mark R
added a comment - - edited The GroovyCallSiteSelector.method method is used to determine the Method object being acted upon. It does consider the type hierarchy but the type hierarchy is ordered by supertypes first . According to this comment it is intentional. As a result all security checking is done on the original method declaring class instead of the class overriding the method.
For the bug in this ticket:
receiver = GitSCM@18657
method = "getBrowser"
args = { Object [0]@18659}
types = {LinkedHashSet@18806} size = 7
0 = { Class @613} " class java.lang. Object "
1 = { Class @6567} " interface hudson.model.Describable"
2 = { Class @6569} " interface hudson.ExtensionPoint"
3 = { Class @8713} " class hudson.scm.SCM"
4 = { Class @610} " interface java.io.Serializable"
5 = { Class @8760} " class hudson.plugins.git.GitSCMBackwardCompatibility"
6 = { Class @8835} " class hudson.plugins.git.GitSCM"
There are two classes that declare the method:
method hudson.scm.SCM getBrowser
method hudson.scm.GitSCM getBrowser // This is whitelisted by an annotation. GitSCM overrides SCM.getBrowser.
Since parent types are checked first it always picks method hudson.scm.SCM getBrowser.
Honestly this seems backwards to me. Whitelisting should default to least privileged and thus should check the most refined child declaring class instead of the most broad parent class. Also, just because I decided a method is okay to use doesn't mean any child overridden implementations are okay. With the current implementation it's not possible to whitelist an overridden implementation of method (or field).
It looks like it has been this way since the initial version (there was a refactor in-between). Based on the commit comment perhaps a copy-paste from the cloudbees-template plugin? I don't have access to that source code so can't trace this any further.
I think the fix for this would be to just change the ordering to check subclasses first? It wouldn't be without impact though. Any whitelists in this situation (method overloading) may need to be updated. It would also mean whitelisting would become more tedious (for example you could no longer blanket whitelist toString() - custom overrides would need to be explicitly whitelisted)
Poking jglick for comment 
Thanks for the issue report. I found a claim in my
JENKINS-42860research job that SCM API needs to allow access to the hudson.scm.SCM.getBrowser method.However, that statement in that test file may be wrong, since I don't see any use of the script security Whitelisted annotation anywhere in Jenkins core.
The workaround is to grant access to the hudson.scm.SCM.getBrowser method. I do that with configuration as code.