Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.xml.stream.XMLInputFactory;
import javax.xml.stream.XMLStreamException;
import javax.xml.stream.XMLStreamReader;
import java.io.InputStream;

public class JaxbXMLMapper {
Expand All @@ -25,6 +28,15 @@ public class JaxbXMLMapper {

private static final JAXBContext jaxbContext = new JAXBContextCreator().create();

/**
* StAX factory hardened against XXE. DTD processing and external entity resolution are
* disabled, so a {@code <!DOCTYPE ...>} declaration (and any external or parameter entity it
* might carry) is rejected rather than resolved. The protection is configured explicitly here
* instead of relying on the defaults of whichever StAX/JAXB provider happens to be on the
* classpath, which could change if a consumer swaps or downgrades that provider.
*/
private static final XMLInputFactory XML_INPUT_FACTORY = createHardenedXmlInputFactory();

/**
* Unmarshalls the configuration from the stream. Uses <code>JAXB</code> for
* this.
Expand All @@ -38,14 +50,26 @@ public class JaxbXMLMapper {
}

try {
final Unmarshaller um = jaxbContext.createUnmarshaller();
@SuppressWarnings("unchecked") final JAXBElement<Configuration> jaxbElement = (JAXBElement<Configuration>) um.unmarshal(stream);
return jaxbElement.getValue();
} catch (JAXBException exception) {
final XMLStreamReader xmlStreamReader = XML_INPUT_FACTORY.createXMLStreamReader(stream);
try {
final Unmarshaller um = jaxbContext.createUnmarshaller();
final JAXBElement<Configuration> jaxbElement = um.unmarshal(xmlStreamReader, Configuration.class);
return jaxbElement.getValue();
} finally {
xmlStreamReader.close();
}
} catch (JAXBException | XMLStreamException exception) {
throw new IllegalStateException("Cannot parse holidays XML file.", exception);
}
}

private static @NonNull XMLInputFactory createHardenedXmlInputFactory() {
final XMLInputFactory factory = XMLInputFactory.newFactory();
factory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
return factory;
}

private static class JAXBContextCreator {
private JAXBContext create() {
return createJAXBContext();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
Expand All @@ -19,6 +21,18 @@
assertThatThrownBy(() -> sut.unmarshallConfiguration(null)).isInstanceOf(IllegalArgumentException.class);
}

@Test
void rejectsXmlWithDoctypeToPreventXxe() {
final String maliciousXml =
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+ "<!DOCTYPE Configuration [<!ENTITY xxe SYSTEM \"file:///etc/passwd\">]>\n"
+ "<Configuration hierarchy=\"test\" description=\"&xxe;\"></Configuration>";

Check warning on line 29 in jollyday-jaxb/src/test/java/de/focus_shift/jollyday/jaxb/JaxbXMLMapperTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace this String concatenation with Text block.

See more on https://sonarcloud.io/project/issues?id=focus-shift_jollyday&issues=AZ8aA4LpHYxAQGlraPvz&open=AZ8aA4LpHYxAQGlraPvz&pullRequest=1311
final InputStream inputStream = new ByteArrayInputStream(maliciousXml.getBytes(StandardCharsets.UTF_8));

assertThatThrownBy(() -> sut.unmarshallConfiguration(inputStream))
.isInstanceOf(IllegalStateException.class);
}

@ParameterizedTest
@ValueSource(strings = {"Holidays_at.xml", "Holidays_de.xml", "Holidays_gb.xml", "Holidays_ua.xml", "Holidays_tr.xml", "Holidays_za.xml"})
void unmarshalRealResource(String holidayFileName) {
Expand Down
Loading