CVE-2021-43288: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: An issue was discovered in ThoughtWorks GoCD before 21.3.0. An attacker in control of a GoCD Agent can plant malicious JavaScript into a failed Job Report.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2022-04-14T13:15Z
Repository: https://github.com/gocd/gocd
Language: Java
Files changed: common/build.gradle, common/src/main/java/com/thoughtworks/go/domain/DirectoryEntries.java, common/src/main/java/com/thoughtworks/go/domain/FileDirectoryEntry.java, common/src/main/java/com/thoughtworks/go/domain/FolderDirectoryEntry.java, common/src/main/java/com/thoughtworks/go/server/presentation/html/HtmlElement.java, common/src/test/java/com/thoughtworks/go/domain/FileDirectoryEntryTest.java, common/src/test/java/com/thoughtworks/go/domain/FolderDirectoryEntryTest.java, common/src/test/java/com/thoughtworks/go/server/presentation/html/HtmlElementTest.java, dependencies.gradle, server/build.gradle
Commit date: 2021-10-19 10:23:57 +0100
Commit message: #000 - Escape filenames in artifact tab

Diff: diff --git a/common/build.gradle b/common/build.gradle
index 9a5e1c8f1c..a3f736e230 100644
--- a/common/build.gradle
+++ b/common/build.gradle
@@ -24,10 +24,11 @@ dependencies {
   api project(':domain')
   api project(':plugin-infra:go-plugin-access')
   api project.deps.dom4j
   api project.deps.apacheHttpMime
   api project.deps.commonsCollections4
+  api project.deps.commonsText
   api project.deps.springTx
   implementation project.deps.jodaTime
   annotationProcessor project.deps.lombok
   compileOnly project.deps.jetBrainsAnnotations
 
diff --git a/common/src/main/java/com/thoughtworks/go/domain/DirectoryEntries.java b/common/src/main/java/com/thoughtworks/go/domain/DirectoryEntries.java
index d366be99ce..708af16db1 100644
--- a/common/src/main/java/com/thoughtworks/go/domain/DirectoryEntries.java
+++ b/common/src/main/java/com/thoughtworks/go/domain/DirectoryEntries.java
@@ -31,11 +31,11 @@ public class DirectoryEntries extends ArrayList<DirectoryEntry> implements HtmlR
     private boolean isArtifactsDeleted;
 
     @Override
     public void render(HtmlRenderer renderer) {
         if (isArtifactsDeleted || isEmpty()) {
-            HtmlElement element = p().content("Artifacts for this job instance are unavailable as they may have been <a href='" +
+            HtmlElement element = p().unsafecontent("Artifacts for this job instance are unavailable as they may have been <a href='" +
                     CurrentGoCDVersion.docsUrl("configuration/delete_artifacts.html") +
                     "' target='blank'>purged by Go</a> or deleted externally. "
                     + "Re-run the stage or job to generate them again.");
             element.render(renderer);
         }
diff --git a/common/src/main/java/com/thoughtworks/go/domain/FileDirectoryEntry.java b/common/src/main/java/com/thoughtworks/go/domain/FileDirectoryEntry.java
index 93ff06a187..54ed5d9652 100644
--- a/common/src/main/java/com/thoughtworks/go/domain/FileDirectoryEntry.java
+++ b/common/src/main/java/com/thoughtworks/go/domain/FileDirectoryEntry.java
@@ -28,11 +28,11 @@ public class FileDirectoryEntry extends DirectoryEntry {
     @Override
     protected HtmlRenderable htmlBody() {
         return HtmlElement.li().content(
             HtmlElement.span(HtmlAttribute.cssClass("artifact")).content(
                 HtmlElement.a(HtmlAttribute.href(getUrl()))
-                        .content(getFileName())
+                        .safecontent(getFileName())
             )
         );
 
     }
 }
diff --git a/common/src/main/java/com/thoughtworks/go/domain/FolderDirectoryEntry.java b/common/src/main/java/com/thoughtworks/go/domain/FolderDirectoryEntry.java
index ac1dc30e7d..7fffa35d80 100644
--- a/common/src/main/java/com/thoughtworks/go/domain/FolderDirectoryEntry.java
+++ b/common/src/main/java/com/thoughtworks/go/domain/FolderDirectoryEntry.java
@@ -39,11 +39,11 @@ public class FolderDirectoryEntry extends DirectoryEntry {
     protected HtmlRenderable htmlBody() {
         return sequence(
                 HtmlElement.div(cssClass("dir-container")).content(
                     HtmlElement.span(cssClass("directory")).content(
                         HtmlElement.a(onclick("BuildDetail.tree_navigator(this)"))
-                                .content(getFileName())
+                                .safecontent(getFileName())
                     )
                 ),
                 HtmlElement.div(cssClass("subdir-container"), style("display:none"))
                         .content(subDirectory)
         );
diff --git a/common/src/main/java/com/thoughtworks/go/server/presentation/html/HtmlElement.java b/common/src/main/java/com/thoughtworks/go/server/presentation/html/HtmlElement.java
index 876d00982c..d55fb63373 100644
--- a/common/src/main/java/com/thoughtworks/go/server/presentation/html/HtmlElement.java
+++ b/common/src/main/java/com/thoughtworks/go/server/presentation/html/HtmlElement.java
@@ -17,10 +17,11 @@ package com.thoughtworks.go.server.presentation.html;
 
 import java.util.ArrayList;
 import java.util.List;
 
 import com.thoughtworks.go.server.presentation.models.HtmlRenderer;
+import org.apache.commons.text.StringEscapeUtils;
 
 public class HtmlElement implements HtmlRenderable {
     public static HtmlElement div(HtmlAttribute... attributes) { return new HtmlElement("div", attributes); }
     public static HtmlElement span(HtmlAttribute... attributes) { return new HtmlElement("span", attributes); }
     public static HtmlElement a(HtmlAttribute... attributes) { return new HtmlElement("a", attributes); }
@@ -35,14 +36,18 @@ public class HtmlElement implements HtmlRenderable {
     private HtmlElement(String elementName, HtmlAttribute... attributes) {
         this.elementName = elementName;
         this.attributes = attributes;
     }
 
-    public HtmlElement content(String body) {
+    public HtmlElement unsafecontent(String body) {
         return content(new TextElement(body));
     }
 
+    public HtmlElement safecontent(String body) {
+        return content(new TextElement(StringEscapeUtils.escapeHtml4(body)));
+    }
+
     public HtmlElement content(HtmlRenderable... elements) {
         for (HtmlRenderable element : elements) {
             addToBody(element);
         }
         return this;
@@ -78,7 +83,6 @@ public class HtmlElement implements HtmlRenderable {
             }
 
             renderer.append("</" + elementName + ">\n");
         }
     }
-
 }
diff --git a/common/src/test/java/com/thoughtworks/go/domain/FileDirectoryEntryTest.java b/common/src/test/java/com/thoughtworks/go/domain/FileDirectoryEntryTest.java
new file mode 100644
index 0000000000..81ab892eee
--- /dev/null
+++ b/common/src/test/java/com/thoughtworks/go/domain/FileDirectoryEntryTest.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2021 ThoughtWorks, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.thoughtworks.go.domain;
+
+import com.thoughtworks.go.server.presentation.models.HtmlRenderer;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+class FileDirectoryEntryTest {
+    @Test
+    void shouldEscapeFilename() {
+        FileDirectoryEntry entry = new FileDirectoryEntry("<div>Hello</div>", "https://www.example.com");
+        HtmlRenderer renderer = new HtmlRenderer("");
+        entry.htmlBody().render(renderer);
+
+        assertTrue(renderer.asString().contains("&lt;div&gt;Hello&lt;/div&gt;"));
+        assertFalse(renderer.asString().contains("<div>Hello</div>"));
+    }
+}
\ No newline at end of file
diff --git a/common/src/test/java/com/thoughtworks/go/domain/FolderDirectoryEntryTest.java b/common/src/test/java/com/thoughtworks/go/domain/FolderDirectoryEntryTest.java
index f109e01c6e..f11e40a7c7 100644
--- a/common/src/test/java/com/thoughtworks/go/domain/FolderDirectoryEntryTest.java
+++ b/common/src/test/java/com/thoughtworks/go/domain/FolderDirectoryEntryTest.java
@@ -13,19 +13,32 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 package com.thoughtworks.go.domain;
 
+import com.thoughtworks.go.server.presentation.models.HtmlRenderer;
 import org.junit.jupiter.api.Test;
 
-import static org.hamcrest.Matchers.hasItem;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.*;
 
 public class FolderDirectoryEntryTest {
     @Test
     public void shouldAddFile() throws Exception {
         DirectoryEntries subDirectory = new DirectoryEntries();
         FolderDirectoryEntry entry = new FolderDirectoryEntry("file", "url", subDirectory);
         entry.addFile("file", "path");
         assertThat(subDirectory, hasItem(new FileDirectoryEntry("file", "path")));
     }
+
+    @Test
+    public void shouldEscapeFolderName() throws Exception {
+        DirectoryEntries subDirectory = new DirectoryEntries();
+        FolderDirectoryEntry entry = new FolderDirectoryEntry("<div>Hello</div>", "url", subDirectory);
+
+        HtmlRenderer renderer = new HtmlRenderer("");
+        entry.htmlBody().render(renderer);
+
+        assertThat(renderer.asString(), containsString("&lt;div&gt;Hello&lt;/div&gt;"));
+        assertThat(renderer.asString(), not(containsString("<div>Hello</div>")));
+    }
 }
diff --git a/common/src/test/java/com/thoughtworks/go/server/presentation/html/HtmlElementTest.java b/common/src/test/java/com/thoughtworks/go/server/presentation/html/HtmlElementTest.java
new file mode 100644
index 0000000000..c3f5bbe862
--- /dev/null
+++ b/common/src/test/java/com/thoughtworks/go/server/presentation/html/HtmlElementTest.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2021 ThoughtWorks, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.thoughtworks.go.server.presentation.html;
+
+import com.thoughtworks.go.server.presentation.models.HtmlRenderer;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+class HtmlElementTest {
+    @Test
+    void shouldEscapeContent() {
+        HtmlElement element = HtmlElement.span().safecontent("<div>Hello</div>");
+        HtmlRenderer renderer = new HtmlRenderer("");
+
+        element.render(renderer);
+
+        assertEquals("<span>\n&lt;div&gt;Hello&lt;/div&gt;\n</span>\n", renderer.asString());
+    }
+}
\ No newline at end of file
diff --git a/dependencies.gradle b/dependencies.gradle
index ddfd55f57e..eba22e67b7 100644
--- a/dependencies.gradle
+++ b/dependencies.gradle
@@ -46,10 +46,11 @@ final Map<String, String> libraries = [
   commonsFileUpload   : 'commons-fileupload:commons-fileupload:1.4',
   commonsIO           : 'commons-io:commons-io:2.11.0',
   commonsLang         : 'commons-lang:commons-lang:2.6',
   commonsLang3        : 'org.apache.commons:commons-lang3:3.12.0',
   commonsPool         : 'org.apache.commons:commons-pool2:2.11.1',
+  commonsText         : 'org.apache.commons:commons-text:1.8',
   dbunit              : 'org.dbunit:dbunit:2.7.2',
   dom4j               : 'dom4j:dom4j:1.6.1',
   ehcache             : 'net.sf.ehcache:ehcache:2.10.9.2',
   felix               : 'org.apache.felix:org.apache.felix.framework:5.6.12',
   freemarker          : 'org.freemarker:freemarker:2.3.31',
@@ -129,10 +130,11 @@ final Map<String, String> v = [
   commonsFileUpload   : versionOf(libraries.commonsFileUpload),
   commonsIO           : versionOf(libraries.commonsIO),
   commonsLang         : versionOf(libraries.commonsLang),
   commonsLang3        : versionOf(libraries.commonsLang3),
   commonsPool         : versionOf(libraries.commonsPool),
+  commonsText         : versionOf(libraries.commonsText),
   dom4j               : versionOf(libraries.dom4j),
   ehcache             : versionOf(libraries.ehcache),
   felix               : versionOf(libraries.felix),
   freemarker          : versionOf(libraries.freemarker),
   gson                : versionOf(libraries.gson),
diff --git a/server/build.gradle b/server/build.gradle
index 19d86780a8..a9ff14709c 100644
--- a/server/build.gradle
+++ b/server/build.gradle
@@ -815,10 +815,11 @@ task verifyWar(type: VerifyJarTask) {
         "commons-digester-${project.versions.commonsDigester}.jar",
         "commons-fileupload-${project.versions.commonsFileUpload}.jar",
         "commons-io-${project.versions.commonsIO}.jar",
         "commons-lang-${project.versions.commonsLang}.jar",
         "commons-lang3-${project.versions.commonsLang3}.jar",
+        "commons-text-${project.versions.commonsText}.jar",
         "commons-pool2-${project.versions.commonsPool}.jar",
         "config-api-${project.version}.jar",
         "config-server-${project.version}.jar",
         "db-${project.version}.jar",
         "db-migration-${project.version}.jar",

Vulnerable code:             HtmlElement element = p().content("Artifacts for this job instance are unavailable as they may have been <a href='" +
                        .content(getFileName())
                                .content(getFileName())
    public HtmlElement content(String body) {

import static org.hamcrest.Matchers.hasItem;
Fixed code:   api project.deps.commonsText
            HtmlElement element = p().unsafecontent("Artifacts for this job instance are unavailable as they may have been <a href='" +
                        .safecontent(getFileName())
                                .safecontent(getFileName())
import org.apache.commons.text.StringEscapeUtils;
    public HtmlElement unsafecontent(String body) {
    public HtmlElement safecontent(String body) {
        return content(new TextElement(StringEscapeUtils.escapeHtml4(body)));
    }

/*
 * Copyright 2021 ThoughtWorks, Inc.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.thoughtworks.go.domain;

import com.thoughtworks.go.server.presentation.models.HtmlRenderer;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.*;

class FileDirectoryEntryTest {
    @Test
    void shouldEscapeFilename() {
        FileDirectoryEntry entry = new FileDirectoryEntry("<div>Hello</div>", "https://www.example.com");
        HtmlRenderer renderer = new HtmlRenderer("");
        entry.htmlBody().render(renderer);

        assertTrue(renderer.asString().contains("&lt;div&gt;Hello&lt;/div&gt;"));
        assertFalse(renderer.asString().contains("<div>Hello</div>"));
    }
}
import com.thoughtworks.go.server.presentation.models.HtmlRenderer;
import static org.hamcrest.Matchers.*;

    @Test
    public void shouldEscapeFolderName() throws Exception {
        DirectoryEntries subDirectory = new DirectoryEntries();
        FolderDirectoryEntry entry = new FolderDirectoryEntry("<div>Hello</div>", "url", subDirectory);

        HtmlRenderer renderer = new HtmlRenderer("");
        entry.htmlBody().render(renderer);

        assertThat(renderer.asString(), containsString("&lt;div&gt;Hello&lt;/div&gt;"));
        assertThat(renderer.asString(), not(containsString("<div>Hello</div>")));
    }
/*
 * Copyright 2021 ThoughtWorks, Inc.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.thoughtworks.go.server.presentation.html;

import com.thoughtworks.go.server.presentation.models.HtmlRenderer;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.*;

class HtmlElementTest {
    @Test
    void shouldEscapeContent() {
        HtmlElement element = HtmlElement.span().safecontent("<div>Hello</div>");
        HtmlRenderer renderer = new HtmlRenderer("");

        element.render(renderer);

        assertEquals("<span>\n&lt;div&gt;Hello&lt;/div&gt;\n</span>\n", renderer.asString());
    }
}
  commonsText         : 'org.apache.commons:commons-text:1.8',
  commonsText         : versionOf(libraries.commonsText),
        "commons-text-${project.versions.commonsText}.jar",