diff --git a/flow/enginesrc/org/labkey/flow/analysis/model/WorkspaceParser.java b/flow/enginesrc/org/labkey/flow/analysis/model/WorkspaceParser.java
index 46a16dcde..0da5834c3 100644
--- a/flow/enginesrc/org/labkey/flow/analysis/model/WorkspaceParser.java
+++ b/flow/enginesrc/org/labkey/flow/analysis/model/WorkspaceParser.java
@@ -23,6 +23,8 @@
import org.apache.xerces.xni.QName;
import org.apache.xerces.xni.XMLLocator;
import org.apache.xerces.xni.XNIException;
+import org.junit.Assert;
+import org.junit.Test;
import org.labkey.api.util.XmlBeansUtil;
import org.w3c.dom.Attr;
import org.w3c.dom.Document;
@@ -41,9 +43,13 @@
import org.xml.sax.helpers.DefaultHandler;
import javax.xml.parsers.SAXParser;
+import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
@@ -147,7 +153,6 @@ static public boolean isFlowJoWorkspace(File file)
}
- /** For debugging. */
static public Document parseXml(InputStream stream) throws Exception
{
DOMParser p = FJDOMParser.create();
@@ -464,6 +469,10 @@ static FJDOMParser create()
setFeature(INCLUDE_IGNORABLE_WHITESPACE, false);
setFeature(NAMESPACES, true);
setFeature(Constants.XERCES_FEATURE_PREFIX + Constants.CONTINUE_AFTER_FATAL_ERROR_FEATURE, true);
+
+ // GH Issue 1523: uploaded workspaces are untrusted.
+ XmlBeansUtil.hardenXercesParser(this);
+
setErrorHandler(new FJErrorHandler());
}
catch (SAXNotSupportedException | SAXNotRecognizedException x)
@@ -520,4 +529,70 @@ public void parse(InputSource inputSource) throws SAXException, IOException
}
}
+ /** GH Issue 1523: uploaded workspaces must not resolve anything beyond the uploaded bytes. */
+ public static class XxeTestCase extends Assert
+ {
+ private static Document parse(String xml) throws Exception
+ {
+ return parseXml(new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8)));
+ }
+
+ @Test
+ public void testExternalGeneralEntityNotResolved() throws Exception
+ {
+ Path secret = Files.createTempFile("wsp-xxe", ".txt");
+ try
+ {
+ Files.writeString(secret, "TOP-SECRET-CONTENTS");
+ Document doc = parse("]>&xx;");
+ assertFalse("External entity resolved the local file", doc.getDocumentElement().getTextContent().contains("TOP-SECRET-CONTENTS"));
+ }
+ finally
+ {
+ Files.deleteIfExists(secret);
+ }
+ }
+
+ @Test
+ public void testExternalDtdNotFetched() throws Exception
+ {
+ // A fetch attempt would fail on this nonexistent path, so parsing cleanly means nothing was fetched
+ Path missing = Files.createTempDirectory("wsp-xxe").resolve("nope.dtd");
+ Document doc = parse("ok");
+ assertEquals("ok", doc.getDocumentElement().getTextContent());
+ }
+
+ @Test
+ public void testEntityExpansionIsBounded() throws Exception
+ {
+ // Deliberately just over the 100,000 expansion limit, not a real billion-laughs: an unguarded parser must
+ // finish this one and fail the assert below.
+ StringBuilder sb = new StringBuilder("");
+ for (int i = 1; i <= 6; i++)
+ {
+ sb.append("");
+ }
+ sb.append("]>&a6;");
+
+ try
+ {
+ parse(sb.toString());
+ fail("Expected the entity expansion limit to stop parsing");
+ }
+ catch (SAXParseException e)
+ {
+ assertTrue("Unexpected failure: " + e.getMessage(), e.getMessage().contains("entity expansions"));
+ }
+ }
+
+ @Test
+ public void testFlowJo7DoctypeStillParses() throws Exception
+ {
+ // FlowJo 7.x emits a bare DOCTYPE, so the declaration itself must stay legal
+ Document doc = parse("");
+ assertEquals("Workspace", doc.getDocumentElement().getTagName());
+ }
+ }
}
diff --git a/flow/src/org/labkey/flow/FlowModule.java b/flow/src/org/labkey/flow/FlowModule.java
index f9fc536d1..ec5c7bed5 100644
--- a/flow/src/org/labkey/flow/FlowModule.java
+++ b/flow/src/org/labkey/flow/FlowModule.java
@@ -48,6 +48,7 @@
import org.labkey.flow.analysis.model.FCSHeader;
import org.labkey.flow.analysis.model.FlowJoWorkspace;
import org.labkey.flow.analysis.model.PopulationName;
+import org.labkey.flow.analysis.model.WorkspaceParser;
import org.labkey.flow.analysis.util.LogicleRangeFunction;
import org.labkey.flow.analysis.web.StatisticSpec;
import org.labkey.flow.analysis.web.SubsetParser;
@@ -324,7 +325,8 @@ public Set getSchemaNames()
SubsetParser.TestLexer.class,
SubsetTests.class,
LogicleRangeFunction.TestCase.class,
- FlowPropertySet.TestCase.class
+ FlowPropertySet.TestCase.class,
+ WorkspaceParser.XxeTestCase.class
);
}