-
Notifications
You must be signed in to change notification settings - Fork 166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add ParserConfiguration#setParsers to blacklist #533
base: master
Are you sure you want to change the base?
Conversation
@uhafner I thought that this might make sense |
src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/blacklist
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine I guess, though oddly specific. What would have made you think this method would be safe to approve to begin with?
There are many thousands of signatures possible in the Java Platform and Jenkins core and various Jenkins plugins, many of which would be security risks. Admins really should never approve anything unless they are Java developers with Jenkins expertise who have studied the method in detail and concluded that it could not be used maliciously (and submitted a PR to this repo to add it to the default whitelist).
The blacklist is intended to capture a handful of signatures which are both especially dangerous, and seem like plausible candidates for approval that an admin might mindlessly approve otherwise. For example GroovyObject.setProperty
might appear as a rejected signature if you try to use reflection-heavy idioms in a sandbox, and if you know nothing about Groovy internals this might sound unremarkable, but in fact it would open up a giant security hole to approve it.
Is this ParserConfiguration.setParsers
something that users would commonly attempt to include in a Jenkinsfile
or other sandboxed script?
@jglick Thanks for your reply! I think it should be part of this list because the plugin is used pretty often and using this function is documented here: This may lead people to use this function. |
Ah, that is not good advice! https://github.com/jenkinsci/script-security-plugin/pull/533/checks?check_run_id=17600899807 is because plugin-supplied signatures cannot be validated in a unit test here. They must be manually verified, then explicitly suppressed from the test as known to be impossible to validate. |
This doesn't make sense to me. In jenkinsci/warnings-ng-plugin#1599 you added |
Add ParserConfiguration#setParsers to blacklist. This function allows to run scripts in the agent-context without further approval by adding them as parsers in the global configuration
-> Privilege Escalation
Testing done
Submitter checklist