CVE-2021-43286: Improper Neutralization of Special Elements used in a Command ('Command Injection') (CWE-77)
value: An issue was discovered in ThoughtWorks GoCD before 21.3.0. An attacker with privileges to create a new pipeline on a GoCD server can abuse a command-line injection in the Git URL "Test Connection" feature to execute arbitrary code.
CWE description: The product constructs all or part of a command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended command when it is sent to a downstream component.
CVSS v3 base score: 8.8
Published: 2022-04-14T13:15Z
Repository: https://github.com/gocd/gocd
Language: Java
Files changed: config/config-api/src/main/java/com/thoughtworks/go/util/command/UrlArgument.java, config/config-api/src/test/java/com/thoughtworks/go/config/materials/git/GitMaterialConfigTest.java, config/config-api/src/test/java/com/thoughtworks/go/config/materials/mercurial/HgMaterialConfigTest.java, config/config-api/src/test/java/com/thoughtworks/go/config/materials/svn/SvnMaterialConfigTest.java, config/config-api/src/test/java/com/thoughtworks/go/config/materials/tfs/TfsMaterialConfigTest.java
Commit date: 2021-10-23 20:11:38 +0100
Commit message: #000 - Allow parameters to be used as URLs

Relates to commit 6fa9fb7a7c91e

Diff: diff --git a/config/config-api/src/main/java/com/thoughtworks/go/util/command/UrlArgument.java b/config/config-api/src/main/java/com/thoughtworks/go/util/command/UrlArgument.java
index bc077f4cef..34206bdf8a 100644
--- a/config/config-api/src/main/java/com/thoughtworks/go/util/command/UrlArgument.java
+++ b/config/config-api/src/main/java/com/thoughtworks/go/util/command/UrlArgument.java
@@ -26,11 +26,11 @@ import java.util.regex.Pattern;
 import static com.thoughtworks.go.util.ExceptionUtils.bombIfNull;
 import static org.apache.commons.lang3.StringUtils.isBlank;
 
 @ConfigAttributeValue(fieldName = "url")
 public class UrlArgument extends CommandArgument {
-    private static final String URL_DUMB_VALIDATION_REGEX = "^[a-zA-Z0-9/].*";
+    private static final String URL_DUMB_VALIDATION_REGEX = "^[a-zA-Z0-9/#].*";
     private static final Pattern pattern = Pattern.compile(URL_DUMB_VALIDATION_REGEX);
 
     protected String url;
 
     public UrlArgument(String url) {
diff --git a/config/config-api/src/test/java/com/thoughtworks/go/config/materials/git/GitMaterialConfigTest.java b/config/config-api/src/test/java/com/thoughtworks/go/config/materials/git/GitMaterialConfigTest.java
index 83e8b9c840..a95564b86b 100644
--- a/config/config-api/src/test/java/com/thoughtworks/go/config/materials/git/GitMaterialConfigTest.java
+++ b/config/config-api/src/test/java/com/thoughtworks/go/config/materials/git/GitMaterialConfigTest.java
@@ -273,10 +273,11 @@ class GitMaterialConfigTest {
             assertTrue(validating(git("-url-not-starting-with-an-alphanumeric-character")).errors().containsKey(GitMaterialConfig.URL));
             assertTrue(validating(git("_url-not-starting-with-an-alphanumeric-character")).errors().containsKey(GitMaterialConfig.URL));
             assertTrue(validating(git("@url-not-starting-with-an-alphanumeric-character")).errors().containsKey(GitMaterialConfig.URL));
 
             assertFalse(validating(git("url-starting-with-an-alphanumeric-character")).errors().containsKey(GitMaterialConfig.URL));
+            assertFalse(validating(git("#{url}")).errors().containsKey(GitMaterialConfig.URL));
         }
 
         private GitMaterialConfig validating(GitMaterialConfig git) {
             git.validate(new ConfigSaveValidationContext(null));
             return git;
diff --git a/config/config-api/src/test/java/com/thoughtworks/go/config/materials/mercurial/HgMaterialConfigTest.java b/config/config-api/src/test/java/com/thoughtworks/go/config/materials/mercurial/HgMaterialConfigTest.java
index be0b88726e..89c3126635 100644
--- a/config/config-api/src/test/java/com/thoughtworks/go/config/materials/mercurial/HgMaterialConfigTest.java
+++ b/config/config-api/src/test/java/com/thoughtworks/go/config/materials/mercurial/HgMaterialConfigTest.java
@@ -298,10 +298,11 @@ class HgMaterialConfigTest {
             assertTrue(validating(hg("-url-not-starting-with-an-alphanumeric-character", "folder")).errors().containsKey(HgMaterialConfig.URL));
             assertTrue(validating(hg("_url-not-starting-with-an-alphanumeric-character", "folder")).errors().containsKey(HgMaterialConfig.URL));
             assertTrue(validating(hg("@url-not-starting-with-an-alphanumeric-character", "folder")).errors().containsKey(HgMaterialConfig.URL));
 
             assertFalse(validating(hg("url-starting-with-an-alphanumeric-character", "folder")).errors().containsKey(HgMaterialConfig.URL));
+            assertFalse(validating(hg("#{url}", "folder")).errors().containsKey(HgMaterialConfig.URL));
         }
 
         private HgMaterialConfig validating(HgMaterialConfig hg) {
             hg.validate(new ConfigSaveValidationContext(null));
             return hg;
diff --git a/config/config-api/src/test/java/com/thoughtworks/go/config/materials/svn/SvnMaterialConfigTest.java b/config/config-api/src/test/java/com/thoughtworks/go/config/materials/svn/SvnMaterialConfigTest.java
index 182d98814e..6ef90d4a3f 100644
--- a/config/config-api/src/test/java/com/thoughtworks/go/config/materials/svn/SvnMaterialConfigTest.java
+++ b/config/config-api/src/test/java/com/thoughtworks/go/config/materials/svn/SvnMaterialConfigTest.java
@@ -166,10 +166,11 @@ class SvnMaterialConfigTest {
             assertTrue(validating(svn("-url-not-starting-with-an-alphanumeric-character", false)).errors().containsKey(SvnMaterialConfig.URL));
             assertTrue(validating(svn("_url-not-starting-with-an-alphanumeric-character", false)).errors().containsKey(SvnMaterialConfig.URL));
             assertTrue(validating(svn("@url-not-starting-with-an-alphanumeric-character", false)).errors().containsKey(SvnMaterialConfig.URL));
 
             assertFalse(validating(svn("url-starting-with-an-alphanumeric-character", false)).errors().containsKey(SvnMaterialConfig.URL));
+            assertFalse(validating(svn("#{url}", false)).errors().containsKey(SvnMaterialConfig.URL));
         }
 
         private SvnMaterialConfig validating(SvnMaterialConfig svn) {
             svn.validate(new ConfigSaveValidationContext(null));
             return svn;
diff --git a/config/config-api/src/test/java/com/thoughtworks/go/config/materials/tfs/TfsMaterialConfigTest.java b/config/config-api/src/test/java/com/thoughtworks/go/config/materials/tfs/TfsMaterialConfigTest.java
index eab7cb5da5..ea5649adad 100644
--- a/config/config-api/src/test/java/com/thoughtworks/go/config/materials/tfs/TfsMaterialConfigTest.java
+++ b/config/config-api/src/test/java/com/thoughtworks/go/config/materials/tfs/TfsMaterialConfigTest.java
@@ -174,10 +174,11 @@ class TfsMaterialConfigTest {
             assertTrue(validating(tfs("-url-not-starting-with-an-alphanumeric-character")).errors().containsKey(TfsMaterialConfig.URL));
             assertTrue(validating(tfs("_url-not-starting-with-an-alphanumeric-character")).errors().containsKey(TfsMaterialConfig.URL));
             assertTrue(validating(tfs("@url-not-starting-with-an-alphanumeric-character")).errors().containsKey(TfsMaterialConfig.URL));
 
             assertFalse(validating(tfs("url-starting-with-an-alphanumeric-character")).errors().containsKey(TfsMaterialConfig.URL));
+            assertFalse(validating(tfs("#{url}")).errors().containsKey(TfsMaterialConfig.URL));
         }
 
         private TfsMaterialConfig validating(TfsMaterialConfig tfs) {
             tfs.validate(new ConfigSaveValidationContext(null));
             return tfs;

Vulnerable code:     private static final String URL_DUMB_VALIDATION_REGEX = "^[a-zA-Z0-9/].*";
Fixed code:     private static final String URL_DUMB_VALIDATION_REGEX = "^[a-zA-Z0-9/#].*";
            assertFalse(validating(git("#{url}")).errors().containsKey(GitMaterialConfig.URL));
            assertFalse(validating(hg("#{url}", "folder")).errors().containsKey(HgMaterialConfig.URL));
            assertFalse(validating(svn("#{url}", false)).errors().containsKey(SvnMaterialConfig.URL));
            assertFalse(validating(tfs("#{url}")).errors().containsKey(TfsMaterialConfig.URL));