Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Port: Prevent XXE injection during CycloneDX validation and parsing #756

Merged
merged 5 commits into from
Jun 25, 2024
Merged

Port: Prevent XXE injection during CycloneDX validation and parsing #756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6cd2d4c
Support ingestion of CycloneDX v1.6 BOMs
nscuro Jun 24, 2024
4a760c5
Address minor change in schema validation output
nscuro Jun 24, 2024
39ac8f7
Fix failing test due to `bom-schema1.4.json` removal
nscuro Jun 24, 2024
3e4ebeb
Fix BOM validation failing when URL contains encoded `[` and `]` char…
nscuro Jun 24, 2024
00c1fbc
Prevent XXE injection during CycloneDX validation and parsing
nscuro Jun 24, 2024
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
14 changes: 1 addition & 13 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
<lib.cpe-parser.version>2.1.0</lib.cpe-parser.version>
<lib.cvss-calculator.version>1.4.3</lib.cvss-calculator.version>
<lib.owasp-rr-calculator.version>1.0.1</lib.owasp-rr-calculator.version>
<lib.cyclonedx-java.version>8.0.3</lib.cyclonedx-java.version>
<lib.cyclonedx-java.version>9.0.4</lib.cyclonedx-java.version>
<lib.jackson.version>2.17.1</lib.jackson.version>
<lib.jackson-databind.version>2.17.1</lib.jackson-databind.version>
<lib.jaxb.runtime.version>2.3.6</lib.jaxb.runtime.version>
Expand Down Expand Up @@ -391,18 +391,6 @@
<version>${lib.liquibase.version}</version>
</dependency>

<!-- Xerces -->
<dependency>
<groupId>xerces</groupId>
<artifactId>xercesImpl</artifactId>
<version>2.12.2</version>
<exclusions>
<exclusion>
<groupId>xml-apis</groupId>
<artifactId>xml-apis</artifactId>
</exclusion>
</exclusions>
</dependency>
<!-- Commons Compress -->
<dependency>
<groupId>org.apache.commons</groupId>
Expand Down
10 changes: 6 additions & 4 deletions src/main/java/org/dependencytrack/parser/cyclonedx/CycloneDXExporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
*/
package org.dependencytrack.parser.cyclonedx;

import org.cyclonedx.BomGeneratorFactory;
import org.cyclonedx.CycloneDxSchema;
import org.cyclonedx.Version;
import org.cyclonedx.exception.GeneratorException;
import org.cyclonedx.generators.BomGeneratorFactory;
import org.cyclonedx.model.Bom;
import org.dependencytrack.model.Component;
import org.dependencytrack.model.Finding;
Expand Down Expand Up @@ -95,10 +95,12 @@ private Bom create(List<Component>components, final List<ServiceComponent> servi
}

public String export(final Bom bom, final Format format) throws GeneratorException {
// TODO: The output version should be user-controllable.

if (Format.JSON == format) {
return BomGeneratorFactory.createJson(CycloneDxSchema.VERSION_LATEST, bom).toJsonString();
return BomGeneratorFactory.createJson(Version.VERSION_15, bom).toJsonString();
} else {
return BomGeneratorFactory.createXml(CycloneDxSchema.VERSION_LATEST, bom).toXmlString();
return BomGeneratorFactory.createXml(Version.VERSION_15, bom).toXmlString();
}
}

Expand Down
50 changes: 33 additions & 17 deletions src/main/java/org/dependencytrack/parser/cyclonedx/CycloneDxValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@
import com.fasterxml.jackson.core.JsonParseException;
import com.fasterxml.jackson.core.JsonToken;
import com.fasterxml.jackson.databind.json.JsonMapper;
import org.codehaus.stax2.XMLInputFactory2;
import org.cyclonedx.CycloneDxSchema;
import org.cyclonedx.Version;
import org.cyclonedx.exception.ParseException;
import org.cyclonedx.parsers.JsonParser;
import org.cyclonedx.parsers.Parser;
import org.cyclonedx.parsers.XmlParser;

import javax.xml.XMLConstants;
import javax.xml.stream.XMLInputFactory;
import javax.xml.stream.XMLStreamException;
import javax.xml.stream.XMLStreamReader;
Expand All @@ -45,12 +45,14 @@
import static org.cyclonedx.CycloneDxSchema.NS_BOM_13;
import static org.cyclonedx.CycloneDxSchema.NS_BOM_14;
import static org.cyclonedx.CycloneDxSchema.NS_BOM_15;
import static org.cyclonedx.CycloneDxSchema.Version.VERSION_10;
import static org.cyclonedx.CycloneDxSchema.Version.VERSION_11;
import static org.cyclonedx.CycloneDxSchema.Version.VERSION_12;
import static org.cyclonedx.CycloneDxSchema.Version.VERSION_13;
import static org.cyclonedx.CycloneDxSchema.Version.VERSION_14;
import static org.cyclonedx.CycloneDxSchema.Version.VERSION_15;
import static org.cyclonedx.CycloneDxSchema.NS_BOM_16;
import static org.cyclonedx.Version.VERSION_10;
import static org.cyclonedx.Version.VERSION_11;
import static org.cyclonedx.Version.VERSION_12;
import static org.cyclonedx.Version.VERSION_13;
import static org.cyclonedx.Version.VERSION_14;
import static org.cyclonedx.Version.VERSION_15;
import static org.cyclonedx.Version.VERSION_16;

/**
* @since 4.11.0
Expand Down Expand Up @@ -92,10 +94,13 @@ public void validate(final byte[] bomBytes) {
}

private FormatAndVersion detectFormatAndSchemaVersion(final byte[] bomBytes) {
final var suppressedExceptions = new ArrayList<Exception>(2);

try {
final CycloneDxSchema.Version version = detectSchemaVersionFromJson(bomBytes);
final Version version = detectSchemaVersionFromJson(bomBytes);
return new FormatAndVersion(Format.JSON, version);
} catch (JsonParseException e) {
suppressedExceptions.add(e);
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Failed to parse BOM as JSON", e);
}
Expand All @@ -104,18 +109,21 @@ private FormatAndVersion detectFormatAndSchemaVersion(final byte[] bomBytes) {
}

try {
final CycloneDxSchema.Version version = detectSchemaVersionFromXml(bomBytes);
final Version version = detectSchemaVersionFromXml(bomBytes);
return new FormatAndVersion(Format.XML, version);
} catch (XMLStreamException e) {
suppressedExceptions.add(e);
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Failed to parse BOM as XML", e);
}
}

throw new InvalidBomException("BOM is neither valid JSON nor XML");
final var exception = new InvalidBomException("BOM is neither valid JSON nor XML");
suppressedExceptions.forEach(exception::addSuppressed);
throw exception;
}

private CycloneDxSchema.Version detectSchemaVersionFromJson(final byte[] bomBytes) throws IOException {
private Version detectSchemaVersionFromJson(final byte[] bomBytes) throws IOException {
try (final com.fasterxml.jackson.core.JsonParser jsonParser = jsonMapper.createParser(bomBytes)) {
JsonToken currentToken = jsonParser.nextToken();
if (currentToken != JsonToken.START_OBJECT) {
Expand All @@ -125,7 +133,7 @@ private CycloneDxSchema.Version detectSchemaVersionFromJson(final byte[] bomByte
.formatted(JsonToken.START_OBJECT.asString(), currentTokenAsString));
}

CycloneDxSchema.Version schemaVersion = null;
Version schemaVersion = null;
while (jsonParser.nextToken() != null) {
final String fieldName = jsonParser.getCurrentName();
if ("specVersion".equals(fieldName)) {
Expand All @@ -138,6 +146,7 @@ private CycloneDxSchema.Version detectSchemaVersionFromJson(final byte[] bomByte
case "1.3" -> VERSION_13;
case "1.4" -> VERSION_14;
case "1.5" -> VERSION_15;
case "1.6" -> VERSION_16;
default ->
throw new InvalidBomException("Unrecognized specVersion %s".formatted(specVersion));
};
Expand All @@ -153,12 +162,18 @@ private CycloneDxSchema.Version detectSchemaVersionFromJson(final byte[] bomByte
}
}

private CycloneDxSchema.Version detectSchemaVersionFromXml(final byte[] bomBytes) throws XMLStreamException {
final XMLInputFactory xmlInputFactory = XMLInputFactory2.newFactory();
private Version detectSchemaVersionFromXml(final byte[] bomBytes) throws XMLStreamException {
final XMLInputFactory xmlInputFactory = XMLInputFactory.newFactory();
xmlInputFactory.setProperty(XMLConstants.FEATURE_SECURE_PROCESSING, true);
// NB: Setting XMLConstants.ACCESS_EXTERNAL_DTD to empty string is recommended by SAST tools,
// but Woodstox does not support it: https://github.com/FasterXML/woodstox/issues/51
// Setting IS_SUPPORTING_EXTERNAL_ENTITIES to false achieves the same:
// https://github.com/FasterXML/woodstox/issues/50#issuecomment-388842419
xmlInputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
final var bomBytesStream = new ByteArrayInputStream(bomBytes);
final XMLStreamReader xmlStreamReader = xmlInputFactory.createXMLStreamReader(bomBytesStream);

CycloneDxSchema.Version schemaVersion = null;
Version schemaVersion = null;
while (xmlStreamReader.hasNext()) {
if (xmlStreamReader.next() == XMLEvent.START_ELEMENT) {
if (!"bom".equalsIgnoreCase(xmlStreamReader.getLocalName())) {
Expand All @@ -177,6 +192,7 @@ private CycloneDxSchema.Version detectSchemaVersionFromXml(final byte[] bomBytes
case NS_BOM_13 -> VERSION_13;
case NS_BOM_14 -> VERSION_14;
case NS_BOM_15 -> VERSION_15;
case NS_BOM_16 -> VERSION_16;
default -> null;
};
}
Expand All @@ -202,7 +218,7 @@ private enum Format {
XML
}

private record FormatAndVersion(Format format, CycloneDxSchema.Version version) {
private record FormatAndVersion(Format format, Version version) {
}

}
36 changes: 20 additions & 16 deletions src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.cyclonedx.model.Metadata;
import org.cyclonedx.model.Swid;
import org.cyclonedx.model.Tool;
import org.cyclonedx.model.license.Expression;
import org.dependencytrack.model.Analysis;
import org.dependencytrack.model.AnalysisJustification;
import org.dependencytrack.model.AnalysisResponse;
Expand Down Expand Up @@ -226,9 +227,9 @@ public static Component convertComponent(final org.cyclonedx.model.Component cdx
}

final var licenseCandidates = new ArrayList<org.cyclonedx.model.License>();
if (cdxComponent.getLicenseChoice() != null) {
if (cdxComponent.getLicenseChoice().getLicenses() != null) {
cdxComponent.getLicenseChoice().getLicenses().stream()
if (cdxComponent.getLicenses() != null) {
if (cdxComponent.getLicenses().getLicenses() != null) {
cdxComponent.getLicenses().getLicenses().stream()
.filter(license -> isNotBlank(license.getId()) || isNotBlank(license.getName()))
.peek(license -> {
// License text can be large, but we don't need it for further processing. Drop it.
Expand All @@ -237,12 +238,13 @@ public static Component convertComponent(final org.cyclonedx.model.Component cdx
.forEach(licenseCandidates::add);
}

if (isNotBlank(cdxComponent.getLicenseChoice().getExpression())) {
final Expression licenseExpression = cdxComponent.getLicenses().getExpression();
if (licenseExpression != null && isNotBlank(licenseExpression.getValue())) {
// If the expression consists of just one license ID, add it as another option.
final var expressionParser = new SpdxExpressionParser();
final SpdxExpression expression = expressionParser.parse(cdxComponent.getLicenseChoice().getExpression());
final SpdxExpression expression = expressionParser.parse(licenseExpression.getValue());
if (!SpdxExpression.INVALID.equals(expression)) {
component.setLicenseExpression(trim(cdxComponent.getLicenseChoice().getExpression()));
component.setLicenseExpression(trim(licenseExpression.getValue()));

if (expression.getSpdxLicenseId() != null) {
final var expressionLicense = new org.cyclonedx.model.License();
Expand All @@ -254,7 +256,7 @@ public static Component convertComponent(final org.cyclonedx.model.Component cdx
LOGGER.warn("""
Encountered invalid license expression "%s" for \
Component{group=%s, name=%s, version=%s, bomRef=%s}; Skipping\
""".formatted(cdxComponent.getLicenseChoice().getExpression(), component.getGroup(),
""".formatted(cdxComponent.getLicenses().getExpression(), component.getGroup(),
component.getName(), component.getVersion(), component.getBomRef()));
}
}
Expand Down Expand Up @@ -626,28 +628,30 @@ public static org.cyclonedx.model.Component convert(final QueryManager qm, final
cycloneComponent.addHash(new Hash(Hash.Algorithm.SHA3_512, component.getSha3_512()));
}

final LicenseChoice licenseChoice = new LicenseChoice();
final LicenseChoice licenses = new LicenseChoice();
if (component.getResolvedLicense() != null) {
final org.cyclonedx.model.License license = new org.cyclonedx.model.License();
license.setId(component.getResolvedLicense().getLicenseId());
license.setUrl(component.getLicenseUrl());
licenseChoice.addLicense(license);
cycloneComponent.setLicenseChoice(licenseChoice);
licenses.addLicense(license);
cycloneComponent.setLicenses(licenses);
} else if (component.getLicense() != null) {
final org.cyclonedx.model.License license = new org.cyclonedx.model.License();
license.setName(component.getLicense());
license.setUrl(component.getLicenseUrl());
licenseChoice.addLicense(license);
cycloneComponent.setLicenseChoice(licenseChoice);
licenses.addLicense(license);
cycloneComponent.setLicenses(licenses);
} else if (StringUtils.isNotEmpty(component.getLicenseUrl())) {
final org.cyclonedx.model.License license = new org.cyclonedx.model.License();
license.setUrl(component.getLicenseUrl());
licenseChoice.addLicense(license);
cycloneComponent.setLicenseChoice(licenseChoice);
licenses.addLicense(license);
cycloneComponent.setLicenses(licenses);
}
if (component.getLicenseExpression() != null) {
licenseChoice.setExpression(component.getLicenseExpression());
cycloneComponent.setLicenseChoice(licenseChoice);
final var licenseExpression = new Expression();
licenseExpression.setValue(component.getLicenseExpression());
licenses.setExpression(licenseExpression);
cycloneComponent.setLicenses(licenses);
}

if (component.getExternalReferences() != null && !component.getExternalReferences().isEmpty()) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/dependencytrack/tasks/BomUploadProcessingTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
import org.apache.commons.collections4.MultiValuedMap;
import org.apache.commons.collections4.multimap.HashSetValuedHashMap;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.cyclonedx.BomParserFactory;
import org.cyclonedx.exception.ParseException;
import org.cyclonedx.parsers.BomParserFactory;
import org.cyclonedx.parsers.Parser;
import org.datanucleus.flush.FlushMode;
import org.datanucleus.store.query.QueryNotUniqueException;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/dependencytrack/tasks/VexUploadProcessingTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import alpine.event.framework.Subscriber;
import alpine.notification.Notification;
import alpine.notification.NotificationLevel;
import org.cyclonedx.BomParserFactory;
import org.cyclonedx.parsers.BomParserFactory;
import org.cyclonedx.parsers.Parser;
import org.dependencytrack.event.VexUploadEvent;
import org.dependencytrack.event.kafka.KafkaEventDispatcher;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/dependencytrack/util/VulnerabilityPolicyUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public class VulnerabilityPolicyUtil {
static {
try {
schema = JsonSchemaFactory.builder(JsonSchemaFactory.getInstance(SpecVersion.VersionFlag.V202012))
.objectMapper(MAPPER).build()
.jsonMapper(MAPPER).build()
.getSchema(resourceToString("/schema/vulnerability-policy-v1.schema.json", StandardCharsets.UTF_8));
} catch (IOException e) {
LOGGER.error("Exception occurred while creating schema for YAML validation", e);
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/dependencytrack/parser/cyclonedx/CycloneDXVexImporterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
package org.dependencytrack.parser.cyclonedx;

import org.assertj.core.api.Assertions;
import org.cyclonedx.BomParserFactory;
import org.cyclonedx.parsers.BomParserFactory;
import org.dependencytrack.PersistenceCapableTest;
import org.dependencytrack.model.Analysis;
import org.dependencytrack.model.AnalysisJustification;
Expand Down
Loading
Loading