[JENKINS-34996] Acknoledge SECURITY-170

jenkinsci · Jun 2, 2016 · 98f1c2f · 98f1c2f
1 parent eebfd72
commit 98f1c2f
Show file tree

Hide file tree

Showing 5 changed files with 175 additions and 2 deletions.
diff --git a/pom.xml b/pom.xml
@@ -15,6 +15,9 @@
 
   <properties>
     <jenkins.version>1.609.3</jenkins.version>
+    <java.level>6</java.level>
+    <!-- TODO: fix findbugs warnings and remove this -->
+    <findbugs.failOnError>false</findbugs.failOnError>
   </properties>
 
   <scm>

diff --git a/src/main/java/hudson/plugins/release/ReleaseWrapper.java b/src/main/java/hudson/plugins/release/ReleaseWrapper.java
@@ -702,7 +702,7 @@ public void doSubmit(StaplerRequest req, StaplerResponse resp) throws IOExceptio
             // schedule release build
             if (!project.scheduleBuild(0, new Cause.UserCause(), 
             		new ReleaseBuildBadgeAction(), 
-            		new ParametersAction(paramValues))) {
+            		new SafeParametersAction(paramValues))) {
             	// TODO redirect to error page?
             }
 

diff --git a/src/main/java/hudson/plugins/release/SafeParametersAction.java b/src/main/java/hudson/plugins/release/SafeParametersAction.java
@@ -0,0 +1,69 @@
+package hudson.plugins.release;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+
+import hudson.EnvVars;
+import hudson.Extension;
+import hudson.model.EnvironmentContributor;
+import hudson.model.ParameterValue;
+import hudson.model.ParametersAction;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+
+@Restricted(NoExternalUse.class)
+public class SafeParametersAction extends ParametersAction {
+
+    private final List<ParameterValue> parameters;
+
+    /**
+     * At this point the list of parameter values is granted to be safe, which is
+     * parameter defined either at top level or release wrapper level.
+     */
+    SafeParametersAction(List<ParameterValue> parameters) {
+        this.parameters = parameters;
+    }
+
+    /**
+     * Returns all parameters allowed by the job (defined as regular job parameters) and
+     * the parameters allowed by release-specific parameters definition.
+     */
+    @Override
+    public List<ParameterValue> getParameters() {
+        return Collections.unmodifiableList(parameters);
+    }
+
+    /**
+     * Returns the parameter if defined as a regular parameters or it is a release-specific parameter defined
+     * by the release wrapper.
+     */
+    @Override
+    public ParameterValue getParameter(String name) {
+        for (ParameterValue p : parameters) {
+            if (p == null) continue;
+            if (p.getName().equals(name)) {
+                return p;
+            }
+        }
+        return null;
+    }
+
+    @Extension
+    public static final class SafeParametersActionEnvironmentContributor extends EnvironmentContributor {
+
+        @Override
+        public void buildEnvironmentFor(Run r, EnvVars envs, TaskListener listener) throws IOException, InterruptedException {
+            SafeParametersAction action = r.getAction(SafeParametersAction.class);
+            if (action != null) {
+                for(ParameterValue p : action.getParameters()) {
+                    envs.put(p.getName(), String.valueOf(p.getValue()));
+                }
+            }
+        }
+    }
+
+}
diff --git a/src/main/resources/hudson/plugins/release/ReleaseWrapper/ReleaseAction/index.jelly b/src/main/resources/hudson/plugins/release/ReleaseWrapper/ReleaseAction/index.jelly
@@ -35,7 +35,7 @@
 	<l:main-panel>
 		<table width="100%">
 			<tr><td>
-			<f:form method="post" action="submit">
+			<f:form method="post" action="submit" name="release-action-form">
                 <j:if test="${it.overrideBuildParameters and !it.buildParameterDefinitions.isEmpty()}">
                     <f:section title="${%Override build parameters}">
                         <j:forEach var="buildParameterDefinition" items="${it.buildParameterDefinitions}">

diff --git a/src/test/java/hudson/plugins/release/TestReleasePluginParameters.java b/src/test/java/hudson/plugins/release/TestReleasePluginParameters.java
@@ -0,0 +1,101 @@
+package hudson.plugins.release;
+
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.util.Arrays;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockBuilder;
+
+import com.gargoylesoftware.htmlunit.html.HtmlForm;
+
+import hudson.Launcher;
+import hudson.model.AbstractBuild;
+import hudson.model.BuildListener;
+import hudson.model.FreeStyleProject;
+import hudson.model.Job;
+import hudson.model.ParameterDefinition;
+import hudson.model.ParametersDefinitionProperty;
+import hudson.model.Result;
+import hudson.model.StringParameterDefinition;
+import hudson.tasks.BuildStep;
+
+public class TestReleasePluginParameters {
+
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+
+    @Test
+    @Issue("JENKINS-34996")
+    public void testReleaseParameters() throws Exception {
+        FreeStyleProject prj = j.createProject(FreeStyleProject.class, "foo");
+        ReleaseWrapper releaseWrapper = new ReleaseWrapper();
+        // Parameter in release specific builder
+        releaseWrapper.setParameterDefinitions(Arrays.asList(new ParameterDefinition [] {
+                new StringParameterDefinition("TEST", "test value") }));
+
+        CheckerBuildStep checkerBuildStep = new CheckerBuildStep();
+        releaseWrapper.setPostSuccessfulBuildSteps(Arrays.asList(new BuildStep [] { checkerBuildStep }));
+        prj.getBuildWrappersList().add(releaseWrapper);
+
+        scheduleReleaseBuild(prj);
+
+        j.waitUntilNoActivity();
+        assertTrue("Build finishes", prj.getLastBuild() != null);
+
+        assertTrue("The parameter must be visible as environment variable", checkerBuildStep.value != null);
+        assertTrue("The parameter value must match", checkerBuildStep.value.equals("test value"));
+        j.assertBuildStatus(Result.SUCCESS, prj.getLastBuild());
+    }
+
+    @Test
+    @Issue("JENKINS-34996") // This test worked before the fix, added just as a verification
+    public void testJobParameters() throws Exception {
+        FreeStyleProject prj = j.createProject(FreeStyleProject.class, "foo");
+        // Parameter at top level (job)
+        prj.addProperty(new ParametersDefinitionProperty(new StringParameterDefinition("TEST","test value")));
+
+        ReleaseWrapper releaseWrapper = new ReleaseWrapper();
+        CheckerBuildStep checkerBuildStep = new CheckerBuildStep();
+        releaseWrapper.setPostSuccessfulBuildSteps(Arrays.asList(new BuildStep [] { checkerBuildStep }));
+        prj.getBuildWrappersList().add(releaseWrapper);
+
+        scheduleReleaseBuild(prj);
+
+        j.waitUntilNoActivity();
+
+        assertTrue("The parameter must be visible as environment variable", checkerBuildStep.value != null);
+        assertTrue("The parameter value must match", checkerBuildStep.value.equals("test value"));
+        j.assertBuildStatus(Result.SUCCESS, prj.getLastBuild());
+    }
+
+    public static class CheckerBuildStep extends MockBuilder {
+
+        public String value;
+
+        public CheckerBuildStep() {
+            super(Result.SUCCESS);
+        }
+
+        @Override
+        public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener)
+                throws InterruptedException, IOException {
+            value = build.getEnvironment(listener).get("TEST");
+            return true;
+        }
+    }
+
+    /**
+     * The release build must be scheduled from UI as all the parameters logic is tied
+     * to {@link ReleaseWrapper.ReleaseAction#doSubmit(org.kohsuke.stapler.StaplerRequest, org.kohsuke.stapler.StaplerResponse)}.
+     */
+    private void scheduleReleaseBuild(Job<?, ?> job) throws Exception {
+        HtmlForm releaseForm = j.createWebClient().getPage(job, "release").getFormByName("release-action-form");
+        j.submit(releaseForm);
+    }
+
+}