CVE-2021-21250: Insertion of Sensitive Information into Externally-Accessible File or Directory (CWE-538)
value: OneDev is an all-in-one devops platform. In OneDev before version 4.0.3, there is a critical vulnerability which may lead to arbitrary file read. When BuildSpec is provided in XML format, the spec is processed by XmlBuildSpecMigrator.migrate(buildSpecString); which processes the XML document without preventing the expansion of external entities. These entities can be configured to read arbitrary files from the file system and dump their contents in the final XML document to be migrated. If the files are dumped in properties included in the YAML file, it will be possible for an attacker to read them. If not, it is possible for an attacker to exfiltrate the contents of these files Out Of Band. This issue was addressed in 4.0.3 by ignoring ENTITY instructions in xml file.
CWE description: The product places sensitive information into files or directories that are accessible to actors who are allowed to have access to the files, but not to the sensitive information.
CVSS v3 base score: 6.5
Published: 2021-01-15T21:15Z
Repository: https://github.com/theonedev/onedev
Language: Java
Files changed: server-core/src/main/java/io/onedev/server/migration/XmlBuildSpecMigrator.java
Commit date: 2020-11-21 21:37:06 +0800
Commit message: Fix XXE injection attack by disabling XML DTD handling
Diff: diff --git a/server-core/src/main/java/io/onedev/server/migration/XmlBuildSpecMigrator.java b/server-core/src/main/java/io/onedev/server/migration/XmlBuildSpecMigrator.java
index 5fecc3ed5ca..9d59206e0eb 100644
--- a/server-core/src/main/java/io/onedev/server/migration/XmlBuildSpecMigrator.java
+++ b/server-core/src/main/java/io/onedev/server/migration/XmlBuildSpecMigrator.java
@@ -8,10 +8,11 @@ import java.util.List;
 
 import org.dom4j.Document;
 import org.dom4j.DocumentException;
 import org.dom4j.Element;
 import org.dom4j.io.SAXReader;
+import org.xml.sax.SAXException;
 import org.yaml.snakeyaml.DumperOptions;
 import org.yaml.snakeyaml.DumperOptions.FlowStyle;
 import org.yaml.snakeyaml.emitter.Emitter;
 import org.yaml.snakeyaml.nodes.MappingNode;
 import org.yaml.snakeyaml.nodes.Node;
@@ -660,12 +661,15 @@ public class XmlBuildSpecMigrator {
 	}
 	
 	public static String migrate(String xml) {
 		Document xmlDoc;
 		try {
-			xmlDoc = new SAXReader().read(new StringReader(xml));
-		} catch (DocumentException e) {
+			SAXReader reader = new SAXReader();
+			// Prevent XXE attack as the xml might be provided by malicious users
+			reader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+			xmlDoc = reader.read(new StringReader(xml));
+		} catch (DocumentException | SAXException e) {
 			throw new RuntimeException(e);
 		}
 		
 		List<NodeTuple> tuples = new ArrayList<>();
 		Node keyNode = new ScalarNode(Tag.STR, "version");

Vulnerable code: 			xmlDoc = new SAXReader().read(new StringReader(xml));
		} catch (DocumentException e) {
Fixed code: import org.xml.sax.SAXException;
			SAXReader reader = new SAXReader();
			// Prevent XXE attack as the xml might be provided by malicious users
			reader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
			xmlDoc = reader.read(new StringReader(xml));
		} catch (DocumentException | SAXException e) {