[SECURITY-2881]

.gitignore

@@ -2,6 +2,8 @@ target
 work
 *.iml
 .idea
-
+/.classpath
+/.project
+/.settings/
 # macOS
 .DS_Store

src/main/java/org/jenkinsci/plugins/workflow/support/steps/input/POSTHyperlinkNote.java

@@ -28,6 +28,10 @@
 import hudson.console.ConsoleAnnotationDescriptor;
 import hudson.console.HyperlinkNote;
 import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import jenkins.model.Jenkins;
@@ -80,7 +84,21 @@ public POSTHyperlinkNote(String url, int length) {
 
     @Override protected String extraAttributes() {
         // TODO perhaps add hoverNotification
-        return " onclick=\"new Ajax.Request('" + url + "'); return false\"";
+        return " onclick=\"new Ajax.Request(decodeURIComponent(atob('" + encodeForJavascript(url) + "'))); return false\"";
+    }
+
+    /**
+     * Encode the String (using URLEncoding and then base64 encoding) so we can safely pass it to javascript where it can be decoded safely.
+     * Javascript strings are UTF-16 and the endianness depends on the platform so we use URL encoding to ensure the String is all 7bit clean ascii and base64 encoding to fix passing any "unsafe" characters.
+     */
+    private static String encodeForJavascript(String str) {
+        // https://developer.mozilla.org/en-US/docs/Glossary/Base64#the_unicode_problem
+        try {
+            String encode = URLEncoder.encode(str, StandardCharsets.UTF_8.name());
+            return Base64.getUrlEncoder().encodeToString(encode.getBytes(StandardCharsets.UTF_8));
+        } catch (UnsupportedEncodingException e) {
+            throw new InternalError("UTF-8 is missing but mandated by the JVM specification", e);
+        }
     }
 
     // TODO why does there need to be a descriptor at all?

src/test/java/org/jenkinsci/plugins/workflow/test/steps/input/POSTHyperlinkNoteTest.java

@@ -0,0 +1,155 @@
+package org.jenkinsci.plugins.workflow.test.steps.input;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.allOf;
+import static org.hamcrest.Matchers.hasProperty;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import java.net.URL;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
+import org.jenkinsci.plugins.workflow.job.WorkflowJob;
+import org.jenkinsci.plugins.workflow.job.WorkflowRun;
+import org.jenkinsci.plugins.workflow.steps.Step;
+import org.jenkinsci.plugins.workflow.steps.StepContext;
+import org.jenkinsci.plugins.workflow.steps.StepDescriptor;
+import org.jenkinsci.plugins.workflow.steps.StepExecution;
+import org.jenkinsci.plugins.workflow.steps.SynchronousStepExecution;
+import org.jenkinsci.plugins.workflow.support.steps.input.POSTHyperlinkNote;
+import org.junit.Ignore;
+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.JenkinsRule.WebClient;
+import org.jvnet.hudson.test.TestExtension;
+import org.kohsuke.stapler.DataBoundConstructor;
+import com.gargoylesoftware.htmlunit.HttpMethod;
+import com.gargoylesoftware.htmlunit.MockWebConnection;
+import com.gargoylesoftware.htmlunit.Page;
+import com.gargoylesoftware.htmlunit.WebRequest;
+import com.gargoylesoftware.htmlunit.html.HtmlAnchor;
+import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import edu.umd.cs.findbugs.annotations.NonNull;
+import hudson.model.ParametersAction;
+import hudson.model.ParametersDefinitionProperty;
+import hudson.model.Result;
+import hudson.model.StringParameterDefinition;
+import hudson.model.StringParameterValue;
+import hudson.model.TaskListener;
+import hudson.model.queue.QueueTaskFuture;
+import jenkins.model.Jenkins;
+
+public class POSTHyperlinkNoteTest {
+
+    @Rule
+    public JenkinsRule jr = new JenkinsRule();
+
+    @Test
+    @Issue("SECURITY-2881")
+    public void urlsAreSafeFromJavascriptInjection() throws Exception {
+        testSanitization("whatever/'+alert(1)+'");
+    }
+
+    @Test
+    @Ignore("webclient does not support unicode URLS and this is passed as /jenkins/whatever/%F0%9F%99%88%F0%9F%99%89%F0%9F%99%8A%F0%9F%98%80%E2%98%BA")
+    public void testPassingMultiByteCharacters() throws Exception {
+        // this is actually illegal in HTML4 but common -> https://www.w3.org/TR/html40/appendix/notes.html#non-ascii-chars
+        // browsers infer the URL from the charset and then encode the escaped characters...
+        testSanitization("whatever/🙈🙉🙊😀☺");
+    }
+
+    @Test
+    public void testPassingSingleByte() throws Exception {
+        testSanitization("whatever/something?withparameter=baa");
+    }
+
+    void testSanitization(String fragment) throws Exception {
+        WorkflowJob project = jr.createProject(WorkflowJob.class);
+        project.setDefinition(new CpsFlowDefinition("security2881(params.TEST_URL)\n", true));
+        project.addProperty(new ParametersDefinitionProperty(new StringParameterDefinition("TEST_URL", "WHOOPS")));
+
+        QueueTaskFuture<WorkflowRun> scheduleBuild = project.scheduleBuild2(0, new ParametersAction(new StringParameterValue("TEST_URL", fragment)));
+        WorkflowRun run = jr.assertBuildStatus(Result.SUCCESS, scheduleBuild);
+        WebClient wc = jr.createWebClient();
+
+        HtmlPage page = wc.getPage(run, "console");
+        HtmlAnchor anchor = page.getAnchorByText("SECURITY-2881");
+        assertThat(anchor, notNullValue());
+        MockWebConnection mwc = new MockWebConnection();
+        mwc.setDefaultResponse("<html><body>Hello</body></html>");
+        wc.setWebConnection(mwc);
+        System.out.println(anchor);
+        Page p = anchor.click();
+
+        // the click executes an ajax request - and so we need to wait until that has completed
+        // ideally we would pass zero here as we have already clicked the javascript should have
+        // started executing - but this is not always the case
+        wc.waitForBackgroundJavaScriptStartingBefore(500);
+
+        // check we have an interaction at the correct place and its not a javascript issue.
+        WebRequest request = mwc.getLastWebRequest();
+        assertThat(request, notNullValue());
+        assertThat(request.getHttpMethod(), is(HttpMethod.POST));
+        URL url = request.getUrl();
+        System.out.println(url.toExternalForm());
+        assertThat(url, allOf(hasProperty("host", is(new URL(jr.jenkins.getConfiguredRootUrl()).getHost())),
+                              hasProperty("file", is(jr.contextPath + '/' + fragment))));
+    }
+
+    public static class Security2881ConsoleStep extends Step {
+
+        private final String urlFragment;
+
+        @DataBoundConstructor
+        public Security2881ConsoleStep(String urlFragment) {
+            this.urlFragment = urlFragment;
+        }
+
+        @Override
+        public StepExecution start(StepContext context) throws Exception {
+            return new Security2881ConsoleStepExecution(context, urlFragment);
+        }
+
+        @TestExtension 
+        public static final class DescriptorImpl extends StepDescriptor {
+
+            @Override public String getFunctionName() {
+                return "security2881";
+            }
+
+            @NonNull
+            @Override public String getDisplayName() {
+                return "Security2881";
+            }
+
+            @Override public Set<? extends Class<?>> getRequiredContext() {
+                return Collections.singleton(TaskListener.class);
+            }
+
+            @Override public String argumentsToString(@NonNull Map<String, Object> namedArgs) {
+                return null;
+            }
+        }
+
+        public static class Security2881ConsoleStepExecution extends SynchronousStepExecution<Void> {
+
+            private final String urlFragment;
+
+            protected Security2881ConsoleStepExecution(StepContext context, String urlFragment) {
+                super(context);
+                this.urlFragment = urlFragment;
+            }
+
+            @Override
+            protected Void run() throws Exception {
+                TaskListener taskListener = getContext().get(TaskListener.class);
+                // use the same URL for CORS.
+                taskListener.getLogger().print(POSTHyperlinkNote.encodeTo(Jenkins.get().getConfiguredRootUrl() + urlFragment, "SECURITY-2881"));
+                return null;
+            }
+        }
+    }
+}