Skip to content

Commit faa49ad

Browse files
committed
[GR-69657] Harden XML processing in MVNDownloader
PullRequest: graal/22136
2 parents d014b72 + 6cdbf2c commit faa49ad

File tree

1 file changed

+34
-3
lines changed

1 file changed

+34
-3
lines changed

sdk/src/org.graalvm.maven.downloader/src/org/graalvm/maven/downloader/MVNDownloader.java

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2023, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -67,6 +67,8 @@
6767
import java.util.logging.Level;
6868
import java.util.stream.Collectors;
6969

70+
import javax.xml.XMLConstants;
71+
import javax.xml.parsers.DocumentBuilder;
7072
import javax.xml.parsers.DocumentBuilderFactory;
7173
import javax.xml.parsers.ParserConfigurationException;
7274

@@ -108,8 +110,7 @@ public void downloadDependencies(String repoUrl, String groupId, String artifact
108110
boolean mvnCentralFallback = !repoUrl.startsWith(DEFAULT_MAVEN_REPO);
109111
byte[] bytes = downloadMavenFile(repoUrl, artifactName, mvnCentralFallback);
110112

111-
var factory = DocumentBuilderFactory.newInstance();
112-
var builder = factory.newDocumentBuilder();
113+
var builder = createSecurePOMParser();
113114
var document = builder.parse(new ByteArrayInputStream(bytes));
114115

115116
// We care only about a very small subset of the POM, and accept even malformed POMs if the
@@ -187,6 +188,36 @@ public void downloadDependencies(String repoUrl, String groupId, String artifact
187188
}
188189
}
189190

191+
/**
192+
* Creates a {@link DocumentBuilder} that is crafted to be safe while still not disallowing
193+
* valid Maven POM files.
194+
*/
195+
private static DocumentBuilder createSecurePOMParser() throws ParserConfigurationException {
196+
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
197+
198+
/*
199+
* Disallow potentially dangerous external entity resolutions. Compatible with Maven because
200+
* POM files do not need external entities.
201+
*/
202+
factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
203+
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
204+
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
205+
206+
/*
207+
* Injecting external XMLs is not allowed by the Maven POM schema, but we can still include
208+
* the rule to err on the side of safety.
209+
*/
210+
factory.setXIncludeAware(false);
211+
212+
/*
213+
* We don't expect legitimate POMs to be huge, so use recommended defaults for max
214+
* processing limits.
215+
*/
216+
factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
217+
218+
return factory.newDocumentBuilder();
219+
}
220+
190221
private static byte[] downloadMavenFile(String repoUrl, String artefactName, boolean fallback) throws IOException, URISyntaxException {
191222
String url = repoUrl + artefactName;
192223
try {

0 commit comments

Comments
 (0)