Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Maven Usage
<dependency>
<groupId>org.cyclonedx</groupId>
<artifactId>cyclonedx-core-java</artifactId>
<version>10.1.0</version>
<version>11.0.1</version>
</dependency>
```

Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<groupId>org.cyclonedx</groupId>
<artifactId>cyclonedx-core-java</artifactId>
<packaging>jar</packaging>
<version>11.1.0-SNAPSHOT</version>
<version>11.0.1-SNAPSHOT</version>

<name>CycloneDX Core (Java)</name>
<description>The CycloneDX core module provides a model representation of the BOM along with utilities to assist in creating, parsing, and validating BOMs.</description>
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/org/cyclonedx/CycloneDxSchema.java
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,9 @@ private Schema getXmlSchema16() throws SAXException {

public Schema getXmlSchema(InputStream... inputStreams) throws SAXException {
final SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this actually prevent XEE? or does it only prevent unintended network access?

what about XMLConstants.FEATURE_SECURE_PROCESSING ?
what about schemaFactory.setExpandEntityReferences(false);?
or similar?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

schemaFactory.setExpandEntityReferences doesn't exist.

The ACCESS_EXTERNAL_* values are a list of protocols which are allowed, setting it to empty makes it disallow all:

I verified that this also prevents file:// access.

I will add FEATURE_SECURE_PROCESSING though, thanks for the tip.

schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
schemaFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
final Source[] schemaFiles = new Source[inputStreams.length];
for (int i = 0; i < inputStreams.length; i++) {
schemaFiles[i] = new StreamSource(inputStreams[i]);
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/cyclonedx/parsers/XmlParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ private Document createSecureDocument(InputSource in) throws ParserConfiguration
DocumentBuilderFactory df = DocumentBuilderFactory.newInstance();
df.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
df.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
df.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
DocumentBuilder builder = df.newDocumentBuilder();
return builder.parse(in);
}
Expand Down
21 changes: 13 additions & 8 deletions src/test/java/org/cyclonedx/BomXmlGeneratorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@
*/
package org.cyclonedx;

import java.nio.charset.StandardCharsets;
import org.apache.commons.io.IOUtils;
import org.cyclonedx.exception.ParseException;
import org.cyclonedx.generators.BomGeneratorFactory;
import org.cyclonedx.generators.json.BomJsonGenerator;
import org.cyclonedx.generators.xml.*;
import org.cyclonedx.generators.xml.BomXmlGenerator;
import org.cyclonedx.model.Attribute;
import org.cyclonedx.model.Bom;
import org.cyclonedx.model.Component;
Expand All @@ -46,20 +45,26 @@
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.w3c.dom.Document;

import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.LinkedList;
import java.util.List;
import java.util.Objects;
import java.util.UUID;
import java.util.stream.Stream;
import java.util.Objects;

import static org.junit.jupiter.api.Assertions.*;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class BomXmlGeneratorTest {

Expand Down Expand Up @@ -670,9 +675,9 @@ public void testIssue408Regression_externalReferenceBom() throws Exception {

@Test
public void testXxeProtection() {
assertThrows(ParseException.class, () -> {
createCommonBomXml("/security/xxe-protection.xml");
});
assertThatExceptionOfType(ParseException.class)
.isThrownBy(() -> createCommonBomXml("/security/xxe-protection.xml"))
.withMessageContaining("not allowed due to restriction set by the accessExternalDTD property");
}

@Test
Expand Down
21 changes: 20 additions & 1 deletion src/test/java/org/cyclonedx/parsers/XmlParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@
*/
package org.cyclonedx.parsers;

import org.apache.commons.io.IOUtils;
import org.cyclonedx.Version;
import org.cyclonedx.exception.ParseException;
import org.cyclonedx.model.Bom;
import org.cyclonedx.model.Component;
import org.cyclonedx.model.Component.Type;
import org.cyclonedx.model.Dependency;
import org.cyclonedx.model.ExternalReference;
import org.cyclonedx.model.License;
import org.cyclonedx.model.LicenseChoice;
import org.cyclonedx.model.OrganizationalEntity;
import org.cyclonedx.model.Pedigree;
Expand Down Expand Up @@ -65,13 +66,16 @@
import org.cyclonedx.model.license.Acknowledgement;
import org.cyclonedx.model.license.Expression;
import org.junit.jupiter.api.Test;

import java.io.File;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
Expand Down Expand Up @@ -746,4 +750,19 @@ public void testIssue492Regression() throws Exception {
final Bom bom = getXmlBom("regression/issue492.xml");
assertEquals(2, bom.getMetadata().getTools().size());
}

@Test
void validateShouldNotBeVulnerableToXxe() throws Exception {
final byte[] bomBytes;
try (final InputStream bomInputStream = getClass().getResourceAsStream("/security/xxe-protection.xml")) {
assertThat(bomInputStream).isNotNull();
bomBytes = IOUtils.toByteArray(bomInputStream);
}

final List<ParseException> validationFailures = new XmlParser().validate(bomBytes);
assertThat(validationFailures).extracting(Throwable::getMessage).allSatisfy(
failureMessage -> assertThat(failureMessage)
.contains("not allowed due to restriction set by the accessExternalDTD property"));
}

}
2 changes: 1 addition & 1 deletion src/test/resources/security/xxe-protection.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE bom [<!ENTITY % sp SYSTEM "https://localhost:1010/test/593ab443b77890d052e55899b16b61a8/raw/3cdd376bba0007145fa2bbacb9abe36cbd5a43ce/file.dtd"> %sp; %param1; %exfil;]>
<!DOCTYPE bom [<!ENTITY % sp SYSTEM "file:///does/not/exist"> %sp; %param1; %exfil;]>
<bom xmlns="http://cyclonedx.org/schema/bom/1.5" serialNumber="urn:uuid:3e671687-395b-41f5-a30f-a58921a69b79">
<components>
<component type="application">
Expand Down