diff --git a/src/org/labkey/targetedms/TargetedMSModule.java b/src/org/labkey/targetedms/TargetedMSModule.java index 925e84906..fd5f6ff36 100644 --- a/src/org/labkey/targetedms/TargetedMSModule.java +++ b/src/org/labkey/targetedms/TargetedMSModule.java @@ -718,6 +718,7 @@ protected void startupAfterSpringConfig(ModuleContext moduleContext) ReplicateLabelMinimizer.TestCase.class, SampleFile.TestCase.class, SkylineAuditLogParser.TestCase.class, + SkylineAuditLogParser.XxeTestCase.class, TargetedMSController.TestCase.class, PrecursorManager.TestCase.class, CrossLinkedPeptideInfo.TestCase.class, diff --git a/src/org/labkey/targetedms/parser/skyaudit/SkylineAuditLogParser.java b/src/org/labkey/targetedms/parser/skyaudit/SkylineAuditLogParser.java index 0c38c0784..0e6e7b980 100644 --- a/src/org/labkey/targetedms/parser/skyaudit/SkylineAuditLogParser.java +++ b/src/org/labkey/targetedms/parser/skyaudit/SkylineAuditLogParser.java @@ -24,6 +24,8 @@ import org.labkey.api.module.Module; import org.labkey.api.module.ModuleLoader; import org.labkey.api.resource.FileResource; +import org.labkey.api.util.ExternalReferenceProbe; +import org.labkey.api.util.FileUtil; import org.labkey.api.util.GUID; import org.labkey.api.util.Path; import org.labkey.api.util.XmlBeansUtil; @@ -31,7 +33,6 @@ import org.labkey.targetedms.parser.XmlUtil; import org.xml.sax.SAXException; -import javax.xml.XMLConstants; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; import javax.xml.transform.stream.StreamSource; @@ -45,6 +46,8 @@ import java.io.IOException; import java.io.InputStream; import java.math.BigDecimal; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.time.format.DateTimeParseException; import java.util.Collections; import java.util.LinkedList; @@ -114,10 +117,10 @@ private void validateXml() throws IOException, SAXException, AuditLogParsingExce try (InputStream schemaStream = new BufferedInputStream(openSchemaInputStream()); InputStream auditLogStream = new BufferedInputStream(new FileInputStream(_file))) { - //prepare validator - SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); + // Use a factory hardened against XXE + SchemaFactory schemaFactory = XmlBeansUtil.schemaFactory(); Schema schema = schemaFactory.newSchema(new StreamSource(schemaStream)); - Validator validator = schema.newValidator(); + Validator validator = XmlBeansUtil.hardenValidator(schema.newValidator()); validator.validate(new StreamSource(auditLogStream)); } } @@ -376,5 +379,65 @@ public void testInvalidXmlFile() throws IOException //TODO: Validate against different files. } + /** + * XXE (CWE-611) coverage for {@link SkylineAuditLogParser#validateXml()}, where the hardening carries the most + * weight: unlike the SAML path nothing guards the uploaded .skyl before it reaches the validator, so if the + * hardening doesn't hold an uploader can make the server fetch a URL of their choosing. + * + *
Separate from {@link TestCase}, which needs a database for its {@code @Before} cleanup.
+ */
+ public static class XxeTestCase extends Assert
+ {
+ /**
+ * {@link #validateXml} resolves the schema first, so an unavailable module resource means it throws before
+ * parsing any XML and every probe-based assertion below goes green while proving nothing.
+ */
+ @Before
+ public void schemaMustBeResolvable()
+ {
+ Module module = ModuleLoader.getInstance().getModule(TargetedMSModule.class);
+ assertNotNull("TargetedMS module must be registered, otherwise validateXml() never validates", module);
+ assertNotNull("Bundled " + SCHEMA_FILE + " must be resolvable, otherwise validateXml() never validates",
+ module.getModuleResolver().lookup(Path.parse(SCHEMA_FILE)));
+ }
+
+ /** Conforming apart from the injected reference, so validation gets far enough to matter. */
+ private static final String AUDIT_LOG =
+ "