[SECURITY-3019]

jenkinsci · Feb 6, 2023 · 0eaf88a · 0eaf88a
1 parent 1be2439
commit 0eaf88a
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 1 deletion.
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java
@@ -3,6 +3,8 @@
 import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.Nullable;
 import hudson.Extension;
+import hudson.Functions;
+import hudson.Util;
 import hudson.model.AutoCompletionCandidates;
 import hudson.model.Describable;
 import hudson.model.Item;
@@ -31,6 +33,7 @@
 import jenkins.model.Jenkins;
 import net.sf.json.JSONArray;
 import net.sf.json.JSONObject;
+import org.apache.commons.lang.StringEscapeUtils;
 import org.apache.commons.lang.StringUtils;
 import org.jenkinsci.plugins.structs.describable.CustomDescribableModel;
 import org.jenkinsci.plugins.structs.describable.DescribableModel;
@@ -289,6 +292,13 @@ public String getContext() {
             return job != null ? job.getFullName() : null;
         }
 
+        @Restricted(DoNotUse.class) // for use from config.jelly
+        public String getContextEncoded() {
+            final String context = getContext();
+            //Functions.jsStringEscape() is also an alternative though the one below escapes more stuff
+            return context != null ? StringEscapeUtils.escapeJavaScript(context) : null;
+        }
+
         public FormValidation doCheckPropagate(@QueryParameter boolean value, @QueryParameter boolean wait) {
             if (!value && !wait) {
                 return FormValidation.warningWithMarkup(Messages.BuildTriggerStep_explicitly_disabling_both_propagate_and_wait());

diff --git a/...esources/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep/config.jelly b/...esources/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep/config.jelly
@@ -43,7 +43,7 @@ THE SOFTWARE.
         <script>
             function loadParams() {
                 var div = $$('params');
-                new Ajax.Request('${descriptor.descriptorUrl}/parameters?job=' + encodeURIComponent($$('${jobFieldId}').value) + '&amp;context=' + encodeURIComponent('${descriptor.context}'), {
+                new Ajax.Request('${descriptor.descriptorUrl}/parameters?job=' + encodeURIComponent($$('${jobFieldId}').value) + '&amp;context=${descriptor.contextEncoded}', {
                     method : 'get',
                     onSuccess : function(x) {
                         div.innerHTML = x.responseText;

diff --git a/...t/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepConfigTest.java b/...t/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepConfigTest.java
@@ -26,16 +26,36 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.collection.IsEmptyCollection.empty;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
+import com.gargoylesoftware.htmlunit.AlertHandler;
+import com.gargoylesoftware.htmlunit.Page;
+import com.gargoylesoftware.htmlunit.html.DomElement;
+import com.gargoylesoftware.htmlunit.html.HtmlElement;
+import com.gargoylesoftware.htmlunit.html.HtmlForm;
+import com.gargoylesoftware.htmlunit.html.HtmlInput;
+import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import com.gargoylesoftware.htmlunit.html.HtmlSelect;
+import com.gargoylesoftware.htmlunit.html.HtmlTextInput;
+import com.gargoylesoftware.htmlunit.javascript.host.html.HTMLInputElement;
+import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.jenkinsci.plugins.workflow.steps.StepConfigTester;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.xml.sax.SAXException;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
 
 public class BuildTriggerStepConfigTest {
 
@@ -65,4 +85,34 @@ public class BuildTriggerStepConfigTest {
                 .getWebResponse().getContentAsString(), containsString("<dt><code>buildVariables</code></dt>"));
     }
 
+    @Test @Issue("SECURITY-3019")
+    public void escapedSnippetConfig() throws IOException, SAXException {
+        final String jobName = "'+alert(123)+'";
+        WorkflowJob j = r.createProject(WorkflowJob.class, jobName);
+        try (JenkinsRule.WebClient webClient = r.createWebClient()) {
+            Alerter alerter = new Alerter();
+            webClient.setAlertHandler(alerter);
+            HtmlPage page = webClient.getPage(j, "pipeline-syntax");
+            final HtmlSelect select = (HtmlSelect)page.getElementsByTagName("select").get(0);
+            page = select.setSelectedAttribute("build: Build a job", true);
+            webClient.waitForBackgroundJavaScript(2000);
+            //final HtmlForm config = page.getFormByName("config");
+            final List<DomElement> inputs = page.getElementsByName("_.job"); //config.getInputsByName("_.job");
+            assertThat(inputs, hasSize(1));
+            final HtmlTextInput jobNameInput = (HtmlTextInput)inputs.get(0);
+            jobNameInput.focus();
+            jobNameInput.blur();
+            assertThat(alerter.messages, empty()); //Fails before the fix
+        }
+
+    }
+
+    static class Alerter implements AlertHandler {
+        List<String> messages = Collections.synchronizedList(new ArrayList<>());
+        @Override
+        public void handleAlert(final Page page, final String message) {
+            messages.add(message);
+        }
+    }
+
 }