[SECURITY-2340]

jenkinsci · May 25, 2021 · c8ed4cb · c8ed4cb
1 parent 039f189
commit c8ed4cb
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 3 deletions.
diff --git a/pom.xml b/pom.xml
@@ -43,6 +43,7 @@
     <jenkins.version>2.204.6</jenkins.version>
     <java.level>8</java.level>
     <checkstyle.version>3.1.1</checkstyle.version>
+    <mockito.version>3.9.0</mockito.version>
   </properties>
 
   <dependencyManagement>
@@ -84,5 +85,11 @@
       <artifactId>xtrigger-lib</artifactId>
       <version>0.34</version>
     </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-core</artifactId>
+      <version>${mockito.version}</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 </project>
diff --git a/src/main/java/org/jenkinsci/plugins/nuget/utils/NugetPackageCheckerVisitor.java b/src/main/java/org/jenkinsci/plugins/nuget/utils/NugetPackageCheckerVisitor.java
@@ -1,8 +1,8 @@
 package org.jenkinsci.plugins.nuget.utils;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Maps;
 import hudson.FilePath;
-import org.jenkinsci.lib.xtrigger.XTriggerLog;
 import org.jenkinsci.plugins.nuget.NugetGlobalConfiguration;
 import org.jenkinsci.plugins.nuget.triggers.logs.TriggerLog;
 import org.w3c.dom.Document;
@@ -18,7 +18,6 @@
 import java.nio.file.Path;
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
-import java.util.Collections;
 import java.util.Map;
 
 /**
@@ -43,7 +42,9 @@ boolean isUpdated() {
         this.configuration = configuration;
         this.preReleaseChecked = preReleaseChecked;
         this.workspaceRoot = workspaceRoot;
-        builder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
+        DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+        factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+        builder = factory.newDocumentBuilder();
     }
 
     @Override
@@ -101,4 +102,9 @@ private String getPackageVersion(FilePath workspaceRoot, String packageName) thr
         command.execute();
         return command.getVersion();
     }
+
+    @VisibleForTesting
+    public Map<String, String> getLatestPackageVersions() {
+        return latestPackageVersions;
+    }
 }
diff --git a/src/test/java/org/jenkinsci/plugins/nuget/utils/NugetPackageCheckerVisitorTest.java b/src/test/java/org/jenkinsci/plugins/nuget/utils/NugetPackageCheckerVisitorTest.java
@@ -0,0 +1,60 @@
+package org.jenkinsci.plugins.nuget.utils;
+
+import org.jenkinsci.plugins.nuget.NugetGlobalConfiguration;
+import org.jenkinsci.plugins.nuget.triggers.logs.TriggerLog;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.xml.sax.SAXParseException;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Path;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+public class NugetPackageCheckerVisitorTest {
+
+    NugetPackageCheckerVisitor visitor;
+    TriggerLog log;
+
+    @Before
+    public void setUp() throws Exception {
+        log = mock(TriggerLog.class);
+        NugetGlobalConfiguration configuration = mock(NugetGlobalConfiguration.class);
+        visitor = new NugetPackageCheckerVisitor(
+            log,
+            configuration,
+            true,
+            null
+        );
+        visitor.getLatestPackageVersions().put("Test", "1.0.0");
+    }
+
+    @Test
+    public void shouldNotBeVulnerableToXxe() throws URISyntaxException, IOException {
+        Path file = getFile("xxe");
+        FileVisitResult fileVisitResult = visitor.visitFile(file, null);
+
+        ArgumentCaptor<SAXParseException> exceptionArgumentCaptor = ArgumentCaptor.forClass(SAXParseException.class);
+        verify(log).errorWhileParsingPackageConfigFile(exceptionArgumentCaptor.capture());
+        SAXParseException exception = exceptionArgumentCaptor.getValue();
+        assertEquals(DOCTYPE_FORBIDDEN_ERROR, exception.getMessage());
+    }
+
+    private Path getFile(String path) throws URISyntaxException {
+        URL url = getClass()
+            .getClassLoader()
+            .getResource("NugetPackageCheckerVisitorTest/" + path + "/packages.config");
+        File file = new File(url.toURI());
+        return file.toPath();
+    }
+
+    final String DOCTYPE_FORBIDDEN_ERROR =
+        "DOCTYPE is disallowed when the feature \"http://apache.org/xml/features/disallow-doctype-decl\" set to true.";
+}
diff --git a/src/test/resources/NugetPackageCheckerVisitorTest/xxe/packages.config b/src/test/resources/NugetPackageCheckerVisitorTest/xxe/packages.config
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!DOCTYPE packages [
+    <!ELEMENT packages (package)>
+    <!ELEMENT package (#PCDATA)>
+    <!ATTLIST package
+        id CDATA #REQUIRED
+        version CDATA #REQUIRED
+        targetFramework CDATA #REQUIRED
+    >
+    <!ENTITY xxe SYSTEM "file:///evil">
+]>
+<packages>
+  <package id="Test" version="1.0.0" targetFramework="net46" >&xxe;</package>
+</packages>