diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovySandbox.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovySandbox.java index 44334b922..eca330807 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovySandbox.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovySandbox.java @@ -126,16 +126,14 @@ public void run() { /** * Runs a script in the sandbox. * You must have used {@link #createSecureCompilerConfiguration} to prepare the Groovy shell. + * Before running a sandboxed script, always {@link org.kohsuke.groovy.sandbox.GroovyInterceptor#register register} and after that make sure to {@link org.kohsuke.groovy.sandbox.GroovyInterceptor#unregister unregister} * @param script a script ready to {@link Script#run}, created for example by {@link GroovyShell#parse(String)} * @param whitelist the whitelist to use, such as {@link Whitelist#all()} * @return the value produced by the script, if any * @throws RejectedAccessException in case an attempted call was not whitelisted */ public static Object run(@Nonnull Script script, @Nonnull final Whitelist whitelist) throws RejectedAccessException { - Whitelist wrapperWhitelist = new ProxyWhitelist( - new ClassLoaderWhitelist(script.getClass().getClassLoader()), - whitelist); - GroovyInterceptor sandbox = new SandboxInterceptor(wrapperWhitelist); + GroovyInterceptor sandbox = createSandbox(script, whitelist); sandbox.register(); try { return script.run(); @@ -144,6 +142,20 @@ public static Object run(@Nonnull Script script, @Nonnull final Whitelist whitel } } + /** + * Prepares a sandbox for a script. + * You must have used {@link #createSecureCompilerConfiguration} to prepare the Groovy shell. + * @param script a script ready to {@link Script#run}, created for example by {@link GroovyShell#parse(String)} + * @param whitelist the whitelist to use, such as {@link Whitelist#all()} + * @return the sandbox for running this script + */ + public static GroovyInterceptor createSandbox(@Nonnull Script script, @Nonnull final Whitelist whitelist) { + Whitelist wrapperWhitelist = new ProxyWhitelist( + new ClassLoaderWhitelist(script.getClass().getClassLoader()), + whitelist); + return new SandboxInterceptor(wrapperWhitelist); + } + private GroovySandbox() {} } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java index db87d307b..4f8954e50 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java @@ -28,6 +28,7 @@ import groovy.lang.Binding; import groovy.lang.GroovyClassLoader; import groovy.lang.GroovyShell; +import groovy.lang.Script; import hudson.Extension; import hudson.PluginManager; import hudson.model.AbstractDescribableImpl; @@ -36,6 +37,8 @@ import hudson.util.FormValidation; import java.beans.Introspector; +import java.io.IOException; +import java.io.Closeable; import java.lang.ref.Reference; import java.lang.ref.WeakReference; import java.lang.reflect.Field; @@ -68,6 +71,7 @@ import org.jenkinsci.plugins.scriptsecurity.scripts.UnapprovedClasspathException; import org.jenkinsci.plugins.scriptsecurity.scripts.UnapprovedUsageException; import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage; +import org.kohsuke.groovy.sandbox.GroovyInterceptor; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.Stapler; @@ -277,6 +281,17 @@ private static void cleanUpObjectStreamClassCaches(@Nonnull Class clazz) thro } } + + /** + * Prepares the Groovy script to be executed. + * @param loader see parameter explanation under {@link #evaluate(ClassLoader, Binding)} + * @param binding see parameter explanation under {@link #evaluate(ClassLoader, Binding)} + * @return the prepared script + */ + public PreparedScript prepare(ClassLoader loader, Binding binding) throws IllegalAccessException, IOException { + return new PreparedScript(loader, binding); + } + /** * Runs the Groovy script, using the sandbox if so configured. * @param loader a class loader for constructing the shell, such as {@link PluginManager#uberClassLoader} (will be augmented by {@link #getClasspath} if nonempty) @@ -287,63 +302,96 @@ private static void cleanUpObjectStreamClassCaches(@Nonnull Class clazz) thro * @throws UnapprovedUsageException in case of a non-sandbox issue * @throws UnapprovedClasspathException in case some unapproved classpath entries were requested */ - @SuppressFBWarnings(value = "DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED", justification = "Managed by GroovyShell.") public Object evaluate(ClassLoader loader, Binding binding) throws Exception { - if (!calledConfiguring) { - throw new IllegalStateException("you need to call configuring or a related method before using GroovyScript"); - } - URLClassLoader urlcl = null; - ClassLoader memoryProtectedLoader = null; - List cp = getClasspath(); - if (!cp.isEmpty()) { - List urlList = new ArrayList(cp.size()); - - for (ClasspathEntry entry : cp) { - ScriptApproval.get().using(entry); - urlList.add(entry.getURL()); - } - - loader = urlcl = new URLClassLoader(urlList.toArray(new URL[urlList.size()]), loader); + try (PreparedScript scriptInstance = prepare(loader, binding)) { + return scriptInstance.run(); } - boolean canDoCleanup = false; + } - try { - loader = GroovySandbox.createSecureClassLoader(loader); + /** + * Allows access to execute a script multiple times without preparation and clean-up overhead. + * While the intricate logic to do this continues to be hidden from users. + */ + public final class PreparedScript implements Closeable { + + private final GroovyShell sh; + private final Script preparedScript; + private ClassLoader memoryProtectedLoader = null; + private GroovyInterceptor scriptSandbox = null; + private URLClassLoader urlcl = null; + private boolean canDoCleanup = false; + + /** + * @see @see SecureGroovyScript#evaluate() + */ + @SuppressFBWarnings(value = "DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED", justification = "Managed by GroovyShell.") + private PreparedScript(ClassLoader loader, Binding binding) throws IllegalAccessException, IOException { + List cp = getClasspath(); + if (!cp.isEmpty()) { + List urlList = new ArrayList(cp.size()); + + for (ClasspathEntry entry : cp) { + ScriptApproval.get().using(entry); + urlList.add(entry.getURL()); + } - Field loaderF = null; - try { - loaderF = GroovyShell.class.getDeclaredField("loader"); - loaderF.setAccessible(true); - canDoCleanup = true; - } catch (NoSuchFieldException nsme) { - LOGGER.log(Level.FINE, "GroovyShell fields have changed, field loader no longer exists -- memory leak fixes won't work"); + loader = urlcl = new URLClassLoader(urlList.toArray(new URL[urlList.size()]), loader); } - GroovyShell sh; - if (sandbox) { - CompilerConfiguration cc = GroovySandbox.createSecureCompilerConfiguration(); - sh = new GroovyShell(loader, binding, cc); + try { + loader = GroovySandbox.createSecureClassLoader(loader); + Field loaderF = null; + try { + loaderF = GroovyShell.class.getDeclaredField("loader"); + loaderF.setAccessible(true); + canDoCleanup = true; + } catch (NoSuchFieldException nsme) { + LOGGER.log(Level.FINE, "GroovyShell fields have changed, field loader no longer exists -- memory leak fixes won't work"); + } - if (canDoCleanup) { - memoryProtectedLoader = new CleanGroovyClassLoader(loader, cc); - loaderF.set(sh, memoryProtectedLoader); + if (sandbox) { + CompilerConfiguration cc = GroovySandbox.createSecureCompilerConfiguration(); + sh = new GroovyShell(loader, binding, cc); + + if (canDoCleanup) { + memoryProtectedLoader = new CleanGroovyClassLoader(loader, cc); + loaderF.set(sh, memoryProtectedLoader); + } + + preparedScript = sh.parse(script); + scriptSandbox = GroovySandbox.createSandbox(preparedScript, Whitelist.all()); + } else { + sh = new GroovyShell(loader, binding); + if (canDoCleanup) { + memoryProtectedLoader = new CleanGroovyClassLoader(loader); + loaderF.set(sh, memoryProtectedLoader); + } + + preparedScript = sh.parse(ScriptApproval.get().using(script, GroovyLanguage.get())); } + } catch (Exception e) { + close(); + throw e; + } + } + public Object run() throws Exception { + if (sandbox) { + scriptSandbox.register(); try { - return GroovySandbox.run(sh.parse(script), Whitelist.all()); + return preparedScript.run(); } catch (RejectedAccessException x) { throw ScriptApproval.get().accessRejected(x, ApprovalContext.create()); + } finally { + scriptSandbox.unregister(); } } else { - sh = new GroovyShell(loader, binding); - if (canDoCleanup) { - memoryProtectedLoader = new CleanGroovyClassLoader(loader); - loaderF.set(sh, memoryProtectedLoader); - } - return sh.evaluate(ScriptApproval.get().using(script, GroovyLanguage.get())); + return preparedScript.run(); } + } - } finally { + @Override + public void close() throws IOException { try { if (canDoCleanup) { cleanUpLoader(memoryProtectedLoader, new HashSet(), new HashSet>()); diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java index 9b1e1a515..395dc3ae1 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java @@ -802,5 +802,32 @@ public void testSandboxClassResolution() throws Exception { assertTrue(e.getMessage().contains("staticMethod java.lang.System gc")); } } - + + @Test public void testSandboxUsesSelectedBinding() throws Exception { + SecureGroovyScript sgs = new SecureGroovyScript("return a++", true, null); + Binding b = new Binding(); + b.setVariable("a", 5); + try(SecureGroovyScript.PreparedScript script = sgs.configuringWithKeyItem().prepare(Jenkins.getInstance().getPluginManager().uberClassLoader, b)) { + Object res = script.run(); + assertTrue((int) res == 5); + assertTrue((int) b.getVariable("a") == 6); + res = script.run(); + assertTrue((int) res == 6); + assertTrue((int) b.getVariable("a") == 7); + } + } + + @Test public void testNonSandboxUsesSelectedBinding() throws Exception { + SecureGroovyScript sgs = new SecureGroovyScript("return a++", false, null); + Binding b = new Binding(); + b.setVariable("a", 5); + try (SecureGroovyScript.PreparedScript script = sgs.configuringWithKeyItem().prepare(Jenkins.getInstance().getPluginManager().uberClassLoader, b)) { + Object res = script.run(); + assertTrue((int) res == 5); + assertTrue((int) b.getVariable("a") == 6); + res = script.run(); + assertTrue((int) res == 6); + assertTrue((int) b.getVariable("a") == 7); + } + } }