[SECURITY-63] Require POST for running Groovy scripts.

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());
     }
 
     /**

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);
             }

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());
     }
 
     /**

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;