[SECURITY-698] SecretBuildWrapper must double up $ twice to undo what…

… Jenkins would otherwise expand.
jenkinsci · Jan 30, 2018 · 0c75238 · 0c75238
1 parent c9f6fb6
commit 0c75238
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 8 deletions.
diff --git a/pom.xml b/pom.xml
@@ -96,6 +96,12 @@
       <version>2.5</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.jenkins-ci.plugins</groupId>
+      <artifactId>durable-task</artifactId>
+      <version>1.13</version>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.jenkins-ci.plugins.workflow</groupId>
       <artifactId>workflow-basic-steps</artifactId>

diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java
@@ -97,7 +97,9 @@ public OutputStream decorateLogger(AbstractBuild build, OutputStream logger) thr
         return new Environment() {
             @Override public void buildEnvVars(Map<String,String> env) {
                 for (MultiBinding.MultiEnvironment e : m) {
-                    env.putAll(e.getValues());
+                    for (Map.Entry<String,String> pair : e.getValues().entrySet()) {
+                        env.put(pair.getKey(), pair.getValue()./* SECURITY-698 */replace("$", "$$$$"));
+                    }
                 }
             }
             @Override public boolean tearDown(AbstractBuild build, BuildListener listener) throws IOException, InterruptedException {

diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStepTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStepTest.java
@@ -126,7 +126,7 @@ public static class Execution extends AbstractSynchronousStepExecution<Void> {
     @Test public void basics() throws Exception {
         final String credentialsId = "creds";
         final String username = "bob";
-        final String password = "s3cr3t";
+        final String password = "s$$cr3t";
         story.addStep(new Statement() {
             @Override public void evaluate() throws Throwable {
                 UsernamePasswordCredentialsImpl c = new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, credentialsId, "sample", username, password);
@@ -137,7 +137,7 @@ public static class Execution extends AbstractSynchronousStepExecution<Void> {
                         + "  withCredentials([usernamePassword(usernameVariable: 'USERNAME', passwordVariable: 'PASSWORD', credentialsId: '" + credentialsId + "')]) {\n"
                         + "    semaphore 'basics'\n"
                         + "    if (isUnix()) {\n"
-                        + "      sh 'echo curl -u $USERNAME:$PASSWORD server > script'\n"
+                        + "      sh 'echo curl -u \"$USERNAME:$PASSWORD\" server > script'\n"
                         + "    } else {\n"
                         + "      bat 'echo curl -u %USERNAME%:%PASSWORD% server > script'\n"
                         + "    }\n"

diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java
@@ -55,13 +55,13 @@ public class SecretBuildWrapperTest {
     @Issue("JENKINS-24805")
     @Test public void maskingFreeStyleSecrets() throws Exception {
         String firstCredentialsId = "creds_1";
-        String firstPassword = "p4ss";
+        String firstPassword = "p4$$";
         StringCredentialsImpl firstCreds = new StringCredentialsImpl(CredentialsScope.GLOBAL, firstCredentialsId, "sample1", Secret.fromString(firstPassword));
 
         CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), firstCreds);
 
         String secondCredentialsId = "creds_2";
-        String secondPassword = "p4ss" + "someMoreStuff";
+        String secondPassword = "p4$$" + "someMoreStuff";
         StringCredentialsImpl secondCreds = new StringCredentialsImpl(CredentialsScope.GLOBAL, secondCredentialsId, "sample2", Secret.fromString(secondPassword));
 
         CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), secondCreds);
@@ -72,16 +72,16 @@ public class SecretBuildWrapperTest {
         FreeStyleProject f = r.createFreeStyleProject();
 
         f.setConcurrentBuild(true);
-        f.getBuildersList().add(Functions.isWindows() ? new BatchFile("echo %PASS_1%") : new Shell("echo $PASS_1"));
-        f.getBuildersList().add(Functions.isWindows() ? new BatchFile("echo %PASS_2%") : new Shell("echo $PASS_2"));
+        f.getBuildersList().add(Functions.isWindows() ? new BatchFile("echo %PASS_1%") : new Shell("echo \"$PASS_1\""));
+        f.getBuildersList().add(Functions.isWindows() ? new BatchFile("echo %PASS_2%") : new Shell("echo \"$PASS_2\""));
         f.getBuildWrappersList().add(wrapper);
 
         r.configRoundtrip((Item)f);
 
         FreeStyleBuild b = r.buildAndAssertSuccess(f);
         r.assertLogNotContains(firstPassword, b);
         r.assertLogNotContains(secondPassword, b);
-        r.assertLogContains("echo ****", b);
+        r.assertLogContains("****", b);
     }
 
     @Issue("JENKINS-24805")