From becdd0d0631a475b1fe8dad888fafaa8c1311568 Mon Sep 17 00:00:00 2001 From: saiteja-in Date: Fri, 13 Mar 2026 14:29:11 +0000 Subject: [PATCH] fix: XXE vulnerability in SPDX parser --- .../parsers/AbstractCLIParser.java | 2 + .../sw360/licenseinfo/parsers/SPDXParser.java | 8 ++- .../licenseinfo/parsers/SPDXParserTest.java | 64 ++++++++++++++++++- 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/backend/licenseinfo/src/main/java/org/eclipse/sw360/licenseinfo/parsers/AbstractCLIParser.java b/backend/licenseinfo/src/main/java/org/eclipse/sw360/licenseinfo/parsers/AbstractCLIParser.java index a4811a31b5..ea7295114b 100644 --- a/backend/licenseinfo/src/main/java/org/eclipse/sw360/licenseinfo/parsers/AbstractCLIParser.java +++ b/backend/licenseinfo/src/main/java/org/eclipse/sw360/licenseinfo/parsers/AbstractCLIParser.java @@ -114,6 +114,8 @@ private Stream streamFromNodeList(NodeList nodes) { protected boolean hasThisXMLRootElement(AttachmentContent content, String rootElementNamespace, String rootElementName, User user, T context) throws TException { XMLInputFactory xmlif = XMLInputFactory.newFactory(); + xmlif.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); + xmlif.setProperty(XMLInputFactory.SUPPORT_DTD, false); XMLStreamReader xmlStreamReader = null; InputStream attachmentStream = null; try { diff --git a/backend/licenseinfo/src/main/java/org/eclipse/sw360/licenseinfo/parsers/SPDXParser.java b/backend/licenseinfo/src/main/java/org/eclipse/sw360/licenseinfo/parsers/SPDXParser.java index aa91500e41..94d75e6096 100644 --- a/backend/licenseinfo/src/main/java/org/eclipse/sw360/licenseinfo/parsers/SPDXParser.java +++ b/backend/licenseinfo/src/main/java/org/eclipse/sw360/licenseinfo/parsers/SPDXParser.java @@ -93,8 +93,14 @@ protected String getUriOfAttachment(AttachmentContent attachmentContent) throws protected Optional openAsSpdx(AttachmentContent attachmentContent, User user, T context) throws SW360Exception { try (InputStream attachmentStream = attachmentConnector.getAttachmentStream(attachmentContent, user, context)) { - DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance(); + DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newDefaultInstance(); dbFactory.setNamespaceAware(true); + dbFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + dbFactory.setFeature("http://xml.org/sax/features/external-general-entities", false); + dbFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + dbFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + dbFactory.setXIncludeAware(false); + dbFactory.setExpandEntityReferences(false); DocumentBuilder dBuilder = dbFactory.newDocumentBuilder(); Document doc = dBuilder.parse(attachmentStream); doc.getDocumentElement().normalize(); diff --git a/backend/licenseinfo/src/test/java/org/eclipse/sw360/licenseinfo/parsers/SPDXParserTest.java b/backend/licenseinfo/src/test/java/org/eclipse/sw360/licenseinfo/parsers/SPDXParserTest.java index c15336314c..067515a329 100644 --- a/backend/licenseinfo/src/test/java/org/eclipse/sw360/licenseinfo/parsers/SPDXParserTest.java +++ b/backend/licenseinfo/src/test/java/org/eclipse/sw360/licenseinfo/parsers/SPDXParserTest.java @@ -19,12 +19,14 @@ import org.eclipse.sw360.datahandler.common.SW360Constants; import org.eclipse.sw360.datahandler.common.SW360Utils; import org.eclipse.sw360.datahandler.couchdb.AttachmentConnector; +import org.eclipse.sw360.datahandler.thrift.SW360Exception; import org.eclipse.sw360.datahandler.thrift.Visibility; import org.eclipse.sw360.datahandler.thrift.attachments.Attachment; import org.eclipse.sw360.datahandler.thrift.attachments.AttachmentContent; import org.eclipse.sw360.datahandler.thrift.attachments.AttachmentType; import org.eclipse.sw360.datahandler.thrift.licenseinfo.LicenseInfo; import org.eclipse.sw360.datahandler.thrift.licenseinfo.LicenseInfoParsingResult; +import org.eclipse.sw360.datahandler.thrift.licenseinfo.LicenseInfoRequestStatus; import org.eclipse.sw360.datahandler.thrift.licenseinfo.LicenseNameWithText; import org.eclipse.sw360.datahandler.thrift.projects.Project; import org.eclipse.sw360.datahandler.thrift.users.User; @@ -41,7 +43,9 @@ import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; +import java.io.ByteArrayInputStream; import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.*; import static org.eclipse.sw360.licenseinfo.TestHelper.*; @@ -138,8 +142,14 @@ public void testAddSPDXContentToCLI(String exampleFile, List expectedLic .setFilename(exampleFile); InputStream input = makeAttachmentContentStream(exampleFile); - DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance(); + DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newDefaultInstance(); dbFactory.setNamespaceAware(true); + dbFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + dbFactory.setFeature("http://xml.org/sax/features/external-general-entities", false); + dbFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + dbFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + dbFactory.setXIncludeAware(false); + dbFactory.setExpandEntityReferences(false); DocumentBuilder dBuilder = dbFactory.newDocumentBuilder(); Document spdxDocument = dBuilder.parse(input); spdxDocument.getDocumentElement().normalize(); @@ -177,4 +187,56 @@ public void testGetLicenseInfo(String exampleFile, List expectedLicenses exampleCopyright, exampleConcludedLicenseIds); } } + + @Test + public void testXXEAttackIsBlocked() throws Exception { + String xxePayload = "\n" + + "\n" + + "]>\n" + + "\n" + + " \n" + + " &xxe;\n" + + " \n" + + ""; + + AttachmentContent attachmentContent = new AttachmentContent() + .setId("xxe-test") + .setFilename("malicious.rdf"); + + // Register content in the store (so AttachmentContentProvider can resolve it), + // then override the mock to return our XXE payload instead of a file resource. + attachmentContentStore.put(attachmentContent); + InputStream xxeStream = new ByteArrayInputStream(xxePayload.getBytes(StandardCharsets.UTF_8)); + Mockito.when(connector.getAttachmentStream(Mockito.eq(attachmentContent), Mockito.any(), Mockito.any())) + .thenReturn(xxeStream); + + // With disallow-doctype-decl=true, the parser rejects DOCTYPE declarations. + // openAsSpdx catches SAXException and throws SW360Exception, but + // getLicenseInfo wraps it and returns FAILURE status. + Attachment attachment = new Attachment() + .setAttachmentContentId("xxe-test") + .setFilename("malicious.rdf") + .setAttachmentType(AttachmentType.COMPONENT_LICENSE_INFO_XML); + + // The parser should reject the XXE payload. With disallow-doctype-decl=true, + // openAsSpdx throws SW360Exception (wrapping SAXException) when it encounters DOCTYPE. + try { + LicenseInfoParsingResult result = parser.getLicenseInfos(attachment, dummyUser, + new Project() + .setVisbility(Visibility.ME_AND_MODERATORS) + .setCreatedBy(dummyUser.getEmail()) + .setAttachments(Collections.singleton(new Attachment().setAttachmentContentId("xxe-test")))) + .stream() + .findFirst() + .orElse(null); + + // If no exception, result must not be SUCCESS + assertTrue("XXE payload should not be parsed successfully", + result == null || result.getStatus() != LicenseInfoRequestStatus.SUCCESS); + } catch (SW360Exception e) { + // Expected — parser rejects DOCTYPE declaration and throws SW360Exception + } + } }