diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java index 99abffa15f412e1997a2ee7ff93fd38c3eb31c75..e3ec868de6f78781b2c441b0f08e38a616cb5f39 100644 --- a/core/src/main/java/hudson/model/Computer.java +++ b/core/src/main/java/hudson/model/Computer.java @@ -50,8 +50,6 @@ import hudson.slaves.RetentionStrategy; import hudson.slaves.WorkspaceList; import hudson.slaves.OfflineCause; import hudson.slaves.OfflineCause.ByCLI; -import hudson.tasks.BuildWrapper; -import hudson.tasks.Publisher; import hudson.util.DaemonThreadFactory; import hudson.util.EditDistance; import hudson.util.ExceptionCatchingThreadFactory; @@ -1130,20 +1128,7 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces } protected void _doScript( StaplerRequest req, StaplerResponse rsp, String view) throws IOException, ServletException { - // ability to run arbitrary script is dangerous - checkPermission(Jenkins.RUN_SCRIPTS); - - String text = req.getParameter("script"); - if(text!=null) { - try { - req.setAttribute("output", - RemotingDiagnostics.executeGroovy(text,getChannel())); - } catch (InterruptedException e) { - throw new ServletException(e); - } - } - - req.getView(this,view).forward(req, rsp); + Jenkins._doScript(req, rsp, req.getView(this, view), getChannel(), getACL()); } /** diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 94066f18cc0aca432788ea9730a98f020b30994c..f172965628f7d5a281f2518a914668238c6051b8 100755 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -260,6 +260,7 @@ import java.io.InputStream; import java.io.PrintWriter; import java.io.StringWriter; import java.net.BindException; +import java.net.HttpURLConnection; import java.net.URL; import java.nio.charset.Charset; import java.security.SecureRandom; @@ -3361,25 +3362,31 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro * Run arbitrary Groovy script. */ public void doScript(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException { - doScript(req, rsp, req.getView(this, "_script.jelly")); + _doScript(req, rsp, req.getView(this, "_script.jelly"), MasterComputer.localChannel, getACL()); } /** * Run arbitrary Groovy script and return result as plain text. */ public void doScriptText(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException { - doScript(req, rsp, req.getView(this, "_scriptText.jelly")); + _doScript(req, rsp, req.getView(this, "_scriptText.jelly"), MasterComputer.localChannel, getACL()); } - private void doScript(StaplerRequest req, StaplerResponse rsp, RequestDispatcher view) throws IOException, ServletException { + /** + * @since 1.509.1 + */ + public static void _doScript(StaplerRequest req, StaplerResponse rsp, RequestDispatcher view, VirtualChannel channel, ACL acl) throws IOException, ServletException { // ability to run arbitrary script is dangerous - checkPermission(RUN_SCRIPTS); + acl.checkPermission(RUN_SCRIPTS); String text = req.getParameter("script"); if (text != null) { + if (!"POST".equals(req.getMethod())) { + throw HttpResponses.error(HttpURLConnection.HTTP_BAD_METHOD, "requires POST"); + } try { req.setAttribute("output", - RemotingDiagnostics.executeGroovy(text, MasterComputer.localChannel)); + RemotingDiagnostics.executeGroovy(text, channel)); } catch (InterruptedException e) { throw new ServletException(e); } diff --git a/maven-plugin/src/main/java/hudson/maven/MavenProbeAction.java b/maven-plugin/src/main/java/hudson/maven/MavenProbeAction.java index dbf4acf2bbd73888e2bf30d5cb9ee15bf239d8de..5efe9ee56cb661e882afd6dbc4de497f6ed49985 100644 --- a/maven-plugin/src/main/java/hudson/maven/MavenProbeAction.java +++ b/maven-plugin/src/main/java/hudson/maven/MavenProbeAction.java @@ -97,21 +97,7 @@ public final class MavenProbeAction implements Action { } public void doScript( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { - // ability to run arbitrary script is dangerous, - // so tie it to the admin access - owner.checkPermission(Jenkins.RUN_SCRIPTS); - - String text = req.getParameter("script"); - if(text!=null) { - try { - req.setAttribute("output", - RemotingDiagnostics.executeGroovy(text,channel)); - } catch (InterruptedException e) { - throw new ServletException(e); - } - } - - req.getView(this,"_script.jelly").forward(req,rsp); + Jenkins._doScript(req, rsp, req.getView(this, "_script.jelly"), channel, owner.getACL()); } /** diff --git a/test/src/test/java/jenkins/model/JenkinsTest.java b/test/src/test/java/jenkins/model/JenkinsTest.java index 81efb770ee6762a9793aed5632d3d8da658a4de9..8f8842f39ae8b935c17ccee7822a7d91d26315c9 100644 --- a/test/src/test/java/jenkins/model/JenkinsTest.java +++ b/test/src/test/java/jenkins/model/JenkinsTest.java @@ -23,6 +23,8 @@ */ package jenkins.model; +import com.gargoylesoftware.htmlunit.HttpMethod; +import com.gargoylesoftware.htmlunit.WebRequestSettings; import com.gargoylesoftware.htmlunit.html.HtmlForm; import hudson.maven.MavenModuleSet; import hudson.maven.MavenModuleSetBuild; @@ -32,6 +34,9 @@ import hudson.security.FullControlOnceLoggedInAuthorizationStrategy; import hudson.util.HttpResponses; import junit.framework.Assert; import hudson.model.FreeStyleProject; +import hudson.security.GlobalMatrixAuthorizationStrategy; +import hudson.security.LegacySecurityRealm; +import hudson.security.Permission; import hudson.util.FormValidation; import org.junit.Test; @@ -41,6 +46,7 @@ import org.jvnet.hudson.test.HudsonTestCase; import org.jvnet.hudson.test.TestExtension; import org.kohsuke.stapler.HttpResponse; import java.net.HttpURLConnection; +import java.net.URL; /** * @author kingfai @@ -209,6 +215,36 @@ public class JenkinsTest extends HudsonTestCase { assertEquals(3,jenkins.getExtensionList(RootAction.class).get(RootActionImpl.class).count); } + public void testDoScript() throws Exception { + jenkins.setSecurityRealm(new LegacySecurityRealm()); + GlobalMatrixAuthorizationStrategy gmas = new GlobalMatrixAuthorizationStrategy() { + @Override public boolean hasPermission(String sid, Permission p) { + return p == Jenkins.RUN_SCRIPTS ? hasExplicitPermission(sid, p) : super.hasPermission(sid, p); + } + }; + gmas.add(Jenkins.ADMINISTER, "alice"); + gmas.add(Jenkins.RUN_SCRIPTS, "alice"); + gmas.add(Jenkins.READ, "bob"); + gmas.add(Jenkins.ADMINISTER, "charlie"); + jenkins.setAuthorizationStrategy(gmas); + WebClient wc = createWebClient(); + wc.login("alice"); + wc.goTo("script"); + wc.assertFails("script?script=System.setProperty('hack','me')", HttpURLConnection.HTTP_BAD_METHOD); + assertNull(System.getProperty("hack")); + WebRequestSettings req = new WebRequestSettings(new URL(wc.getContextPath() + "script?script=System.setProperty('hack','me')"), HttpMethod.POST); + wc.getPage(wc.addCrumb(req)); + assertEquals("me", System.getProperty("hack")); + wc.assertFails("scriptText?script=System.setProperty('hack','me')", HttpURLConnection.HTTP_BAD_METHOD); + req = new WebRequestSettings(new URL(wc.getContextPath() + "scriptText?script=System.setProperty('huck','you')"), HttpMethod.POST); + wc.getPage(wc.addCrumb(req)); + assertEquals("you", System.getProperty("huck")); + wc.login("bob"); + wc.assertFails("script", HttpURLConnection.HTTP_FORBIDDEN); + wc.login("charlie"); + wc.assertFails("script", HttpURLConnection.HTTP_FORBIDDEN); + } + @TestExtension("testUnprotectedRootAction") public static class RootActionImpl implements UnprotectedRootAction { private int count;