Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
920aa68
Avoid inline display of HTML/JS bitstreams. Add JS to list of known …
tdonohue Jun 6, 2024
941baf3
For additional security, ensure "unknown" formats are always download…
tdonohue Jun 10, 2024
31c7db6
To avoid misconfiguration, hardcode HTML, XML, RDF, JS to download on…
tdonohue Jun 10, 2024
1b71dac
Potential fix for code scanning alert no. 30: Resolving XML external …
tdonohue Apr 29, 2025
762449c
Cannot disable DTDs with PubMed, so instead disallow external entitie…
tdonohue Apr 29, 2025
250cb5e
Potential fix for code scanning alert no. 3549: Arbitrary file access…
tdonohue May 9, 2025
81ab9d5
Safe and consistent XML entity handling in parsers
kshepherd Jul 2, 2025
ea2d1d7
EPO and PubMed only need to allow for DOCTYPEs. All other XML securi…
tdonohue Jul 2, 2025
d3da7a9
Allow trusted XML builder to enforce base path for entities
kshepherd Jul 10, 2025
ac55d9e
Enforce path traversal check on import subdir (pre-processing)
kshepherd Jul 13, 2025
02ad2bf
Re-add file separator to normalized SAF item path
kshepherd Jul 14, 2025
0f17227
Remove unused imports
kshepherd Jul 14, 2025
00c1cf0
Fix missing XMLUtils imports
kshepherd Jul 14, 2025
4e0bf93
fix: prevent path traversal in SAF import
MMilosz Jul 4, 2025
e63e0bd
Enforce bitstream path to be within (fs) bitstore base on get
kshepherd Jul 9, 2025
e5e4722
return existing File constructed and validated for bitstream
kshepherd Jul 9, 2025
b13a8ae
Fix line length in DSBitstore log
kshepherd Jul 14, 2025
753f7c3
ORE aggregated resource URI validation
kshepherd May 16, 2026
73f9d73
Velocity and template safety for Email and LDN messages
kshepherd May 5, 2026
9a1ae87
Better null checking in allowed config props
kshepherd May 16, 2026
606d5ff
Access configurationService at runtime, not rely on class setup
kshepherd May 27, 2026
11bd60e
Remove strict mode Velocity engine configuration (allow nulls)
kshepherd May 27, 2026
ec51a10
Filter requests for JSPs or traversal
kshepherd May 16, 2026
6044a23
Add additional logging to GlobalRequestSecurityFilter
kshepherd May 16, 2026
22ec84b
Fix import order
kshepherd May 27, 2026
90c625c
Update sitemap traversal test expectations
kshepherd May 27, 2026
596dc9c
Backport GlobalRequestSecurityFilter for javax
kshepherd May 27, 2026
99f5e81
Add secure file access methods
kshepherd May 27, 2026
237a61b
Backport Curation I/O using secure file access
kshepherd May 27, 2026
5795118
Curation config support for allowed base paths
kshepherd May 27, 2026
ada55c6
Move curation -r reporter param to CLI only
kshepherd May 26, 2026
0cf37a2
Fix import order
kshepherd May 27, 2026
df08846
Ignore CurationScriptIT -T taskFile tests, to rewrite w/ CLI
kshepherd May 27, 2026
9247d01
Move taskfile -T option to CLI script config only
kshepherd May 27, 2026
9f45b37
Align webui.content_disposition_format defaults with upstream 7.6.7
milanmajchrak Jun 11, 2026
2698f83
Fix failing IT by increasing number of formats by one
milanmajchrak Jun 11, 2026
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 @@ -10,14 +10,14 @@
import java.io.File;
import java.io.IOException;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.transform.TransformerException;
import javax.xml.xpath.XPath;
import javax.xml.xpath.XPathConstants;
import javax.xml.xpath.XPathExpressionException;
import javax.xml.xpath.XPathFactory;

import org.dspace.app.util.XMLUtils;
import org.w3c.dom.Document;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
Expand Down Expand Up @@ -49,8 +49,9 @@ private RegistryImporter() { }
*/
public static Document loadXML(String filename)
throws IOException, ParserConfigurationException, SAXException {
DocumentBuilder builder = DocumentBuilderFactory.newInstance()
.newDocumentBuilder();
// This XML builder will *not* disable external entities as XML
// registries are considered trusted content
DocumentBuilder builder = XMLUtils.getTrustedDocumentBuilder();

Document document = builder.parse(new File(filename));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import java.util.ArrayList;
import java.util.Arrays;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.transform.TransformerException;
import javax.xml.xpath.XPath;
Expand All @@ -22,6 +21,7 @@
import javax.xml.xpath.XPathFactory;

import org.apache.logging.log4j.Logger;
import org.dspace.app.util.XMLUtils;
import org.dspace.authorize.AuthorizeException;
import org.dspace.content.BitstreamFormat;
import org.dspace.content.factory.ContentServiceFactory;
Expand Down Expand Up @@ -210,8 +210,9 @@ private static void loadFormat(Context context, Node node)
*/
private static Document loadXML(String filename) throws IOException,
ParserConfigurationException, SAXException {
DocumentBuilder builder = DocumentBuilderFactory.newInstance()
.newDocumentBuilder();
// This XML builder will *not* disable external entities as XML
// registries are considered trusted content
DocumentBuilder builder = XMLUtils.getTrustedDocumentBuilder();

return builder.parse(new File(filename));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import java.util.List;
import java.util.Map;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.transform.TransformerException;
import javax.xml.xpath.XPath;
Expand All @@ -43,6 +42,7 @@
import org.apache.commons.cli.Options;
import org.apache.commons.cli.ParseException;
import org.apache.commons.lang3.StringUtils;
import org.dspace.app.util.XMLUtils;
import org.dspace.authorize.AuthorizeException;
import org.dspace.content.Collection;
import org.dspace.content.Community;
Expand Down Expand Up @@ -613,8 +613,8 @@ private static String validateCollections(NodeList collections, int level)
*/
private static org.w3c.dom.Document loadXML(InputStream input)
throws IOException, ParserConfigurationException, SAXException {
DocumentBuilder builder = DocumentBuilderFactory.newInstance()
.newDocumentBuilder();
// This builder factory does not disable external DTD, entities, etc.
DocumentBuilder builder = XMLUtils.getTrustedDocumentBuilder();

org.w3c.dom.Document document = builder.parse(input);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.io.OutputStream;
import java.io.PrintWriter;
import java.net.URL;
import java.nio.file.Path;
import java.sql.SQLException;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
Expand All @@ -50,7 +51,6 @@
import java.util.zip.ZipFile;
import javax.mail.MessagingException;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.transform.TransformerException;
import javax.xml.xpath.XPath;
Expand All @@ -70,6 +70,7 @@
import org.dspace.app.itemimport.service.ItemImportService;
import org.dspace.app.util.LocalSchemaFilenameFilter;
import org.dspace.app.util.RelationshipUtils;
import org.dspace.app.util.XMLUtils;
import org.dspace.authorize.AuthorizeException;
import org.dspace.authorize.ResourcePolicy;
import org.dspace.authorize.service.AuthorizeService;
Expand Down Expand Up @@ -189,6 +190,8 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea
@Autowired(required = true)
protected ClarinLicenseResourceMappingService clarinLicenseResourceMappingService;

protected DocumentBuilder builder;

protected String tempWorkDir;

protected boolean isTest = false;
Expand Down Expand Up @@ -783,15 +786,22 @@ protected Item addItem(Context c, List<Collection> mycollections, String path,
myitem = wi.getItem();
}

// normalize and validate path to make sure itemname doesn't contain path traversal
Path itemPath = new File(path + File.separatorChar + itemname + File.separatorChar)
.toPath().normalize();
if (!itemPath.startsWith(path)) {
throw new IOException("Illegal item metadata path: '" + itemPath);
}
// Normalization chops off the last separator, and we need to put it back
String itemPathDir = itemPath.toString() + File.separatorChar;

// now fill out dublin core for item
loadMetadata(c, myitem, path + File.separatorChar + itemname
+ File.separatorChar);
loadMetadata(c, myitem, itemPathDir);

// and the bitstreams from the contents file
// process contents file, add bistreams and bundles, return any
// non-standard permissions
List<String> options = processContentsFile(c, myitem, path
+ File.separatorChar + itemname, "contents");
List<String> options = processContentsFile(c, myitem, itemPathDir, "contents");

if (useWorkflow) {
// don't process handle file
Expand All @@ -809,8 +819,7 @@ protected Item addItem(Context c, List<Collection> mycollections, String path,
}
} else {
// only process handle file if not using workflow system
String myhandle = processHandleFile(c, myitem, path
+ File.separatorChar + itemname, "handle");
String myhandle = processHandleFile(c, myitem, itemPathDir, "handle");

// put item in system
if (!isTest) {
Expand Down Expand Up @@ -1037,6 +1046,34 @@ protected void addDCValue(Context c, Item i, String schema, Node n)
}
}

/**
* Ensures a file path does not attempt to access files outside the designated parent directory.
*
* @param parentDir The absolute path to the parent directory that should contain the file
* @param fileName The name or path of the file to validate
* @throws IOException If an error occurs while resolving canonical paths, or the file path attempts
* to access a location outside the parent directory
*/
private void validateFilePath(String parentDir, String fileName) throws IOException {
File parent = new File(parentDir);
File file = new File(fileName);

// If the fileName is not an absolute path, we resolve it against the parentDir
if (!file.isAbsolute()) {
file = new File(parent, fileName);
}

String parentCanonicalPath = parent.getCanonicalPath();
String fileCanonicalPath = file.getCanonicalPath();

if (!fileCanonicalPath.startsWith(parentCanonicalPath)) {
log.error("File path outside of canonical root requested: fileCanonicalPath={} does not begin " +
"with parentCanonicalPath={}", fileCanonicalPath, parentCanonicalPath);
throw new IOException("Illegal file path '" + fileName + "' encountered. This references a path " +
"outside of the import package. Please see the system logs for more details.");
}
}

/**
* Read the collections file inside the item directory. If there
* is one and it is not empty return a list of collections in
Expand Down Expand Up @@ -1237,6 +1274,7 @@ protected List<String> processContentsFile(Context c, Item i, String path,
sDescription = sDescription.replaceFirst("description:", "");
}

validateFilePath(path, sFilePath);
registerBitstream(c, i, iAssetstore, sFilePath, sBundle, sDescription);
logInfo("\tRegistering Bitstream: " + sFilePath
+ "\tAssetstore: " + iAssetstore
Expand Down Expand Up @@ -1450,6 +1488,7 @@ protected void processContentFileEntry(Context c, Item i, String path,
return;
}

validateFilePath(path, fileName);
String fullpath = path + File.separatorChar + fileName;

// get an input stream
Expand Down Expand Up @@ -1922,9 +1961,7 @@ protected String getStringValue(Node node) {
*/
protected Document loadXML(String filename) throws IOException,
ParserConfigurationException, SAXException {
DocumentBuilder builder = DocumentBuilderFactory.newInstance()
.newDocumentBuilder();

DocumentBuilder builder = XMLUtils.getDocumentBuilder();
return builder.parse(new File(filename));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
import java.util.Iterator;
import java.util.List;
import java.util.UUID;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.transform.Transformer;
import javax.xml.transform.TransformerConfigurationException;
Expand All @@ -33,6 +31,7 @@

import org.apache.logging.log4j.Logger;
import org.dspace.app.util.LocalSchemaFilenameFilter;
import org.dspace.app.util.XMLUtils;
import org.dspace.authorize.AuthorizeException;
import org.dspace.content.DSpaceObject;
import org.dspace.content.Item;
Expand All @@ -52,7 +51,6 @@ public class ItemArchive {

public static final String DUBLIN_CORE_XML = "dublin_core.xml";

protected static DocumentBuilder builder = null;
protected Transformer transformer = null;

protected List<DtoMetadata> dtomList = null;
Expand Down Expand Up @@ -95,14 +93,14 @@ public static ItemArchive create(Context context, File dir, String itemField)
InputStream is = null;
try {
is = new FileInputStream(new File(dir, DUBLIN_CORE_XML));
itarch.dtomList = MetadataUtilities.loadDublinCore(getDocumentBuilder(), is);
itarch.dtomList = MetadataUtilities.loadDublinCore(XMLUtils.getDocumentBuilder(), is);

//The code to search for local schema files was copied from org.dspace.app.itemimport
// .ItemImportServiceImpl.java
File file[] = dir.listFiles(new LocalSchemaFilenameFilter());
for (int i = 0; i < file.length; i++) {
is = new FileInputStream(file[i]);
itarch.dtomList.addAll(MetadataUtilities.loadDublinCore(getDocumentBuilder(), is));
itarch.dtomList.addAll(MetadataUtilities.loadDublinCore(XMLUtils.getDocumentBuilder(), is));
}
} finally {
if (is != null) {
Expand All @@ -126,14 +124,6 @@ public static ItemArchive create(Context context, File dir, String itemField)
return itarch;
}

protected static DocumentBuilder getDocumentBuilder()
throws ParserConfigurationException {
if (builder == null) {
builder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
}
return builder;
}

/**
* Getter for Transformer
*
Expand Down Expand Up @@ -318,7 +308,7 @@ public void writeUndo(File undoDir)

try {
out = new FileOutputStream(new File(dir, "dublin_core.xml"));
Document doc = MetadataUtilities.writeDublinCore(getDocumentBuilder(), undoDtomList);
Document doc = MetadataUtilities.writeDublinCore(XMLUtils.getDocumentBuilder(), undoDtomList);
MetadataUtilities.writeDocument(doc, getTransformer(), out);

// if undo has delete bitstream
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.apache.commons.cli.ParseException;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.dspace.app.util.XMLUtils;
import org.dspace.core.Context;
import org.dspace.scripts.DSpaceRunnable;
import org.dspace.scripts.configuration.ScriptConfiguration;
Expand Down Expand Up @@ -308,7 +309,7 @@ public static Document getConfig(DSpaceKernelImpl kernelImpl) {
String config = kernelImpl.getConfigurationService().getProperty("dspace.dir") +
System.getProperty("file.separator") + "config" +
System.getProperty("file.separator") + "launcher.xml";
SAXBuilder saxBuilder = new SAXBuilder();
SAXBuilder saxBuilder = XMLUtils.getSAXBuilder();
Document doc = null;
try {
doc = saxBuilder.build(config);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.Logger;
import org.dspace.app.sfx.service.SFXFileReaderService;
import org.dspace.app.util.XMLUtils;
import org.dspace.content.DCPersonName;
import org.dspace.content.Item;
import org.dspace.content.MetadataValue;
Expand Down Expand Up @@ -79,9 +80,9 @@ public Document parseFile(String fileName) {
log.info("Parsing XML file... " + fileName);
DocumentBuilder docBuilder;
Document doc = null;
DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance();
docBuilderFactory.setIgnoringElementContentWhitespace(true);
try {
DocumentBuilderFactory docBuilderFactory = XMLUtils.getDocumentBuilderFactory();
docBuilderFactory.setIgnoringElementContentWhitespace(true);
docBuilder = docBuilderFactory.newDocumentBuilder();
} catch (ParserConfigurationException e) {
log.error("Wrong parser configuration: " + e.getMessage());
Expand Down
17 changes: 9 additions & 8 deletions dspace-api/src/main/java/org/dspace/app/util/DCInputsReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import java.util.Map;
import javax.servlet.ServletException;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.FactoryConfigurationError;

import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -129,15 +128,17 @@ private void buildInputs(String fileName)
valuePairs = new HashMap<String, List<String>>();
complexDefinitions = new DCInput.ComplexDefinitions(valuePairs);

String uri = "file:" + new File(fileName).getAbsolutePath();
File inputFile = new File(fileName);
String inputFileDir = inputFile.toPath().normalize().getParent().toString();

try {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory.setValidating(false);
factory.setIgnoringComments(true);
factory.setIgnoringElementContentWhitespace(true);
String uri = "file:" + inputFile.getAbsolutePath();

DocumentBuilder db = factory.newDocumentBuilder();
try {
// This document builder will *not* disable external
// entities as they can be useful in managing large forms, but
// it will restrict them to be within the directory that the
// current input form XML file exists (or a sub-directory)
DocumentBuilder db = XMLUtils.getTrustedDocumentBuilder(inputFileDir);
Document doc = db.parse(uri);
doNodes(doc);
checkValues();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import java.io.IOException;
import java.sql.SQLException;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;

import org.apache.commons.cli.CommandLine;
Expand Down Expand Up @@ -105,8 +104,9 @@ private void run(String fileLocation) throws SQLException, AuthorizeException {
private void parseXMLToRelations(Context context, String fileLocation) throws AuthorizeException {
try {
File fXmlFile = new File(fileLocation);
DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance();
DocumentBuilder dBuilder = dbFactory.newDocumentBuilder();
// This XML builder will allow external entities, so the relationship types XML should
// be considered trusted by administrators
DocumentBuilder dBuilder = XMLUtils.getTrustedDocumentBuilder();
Document doc = dBuilder.parse(fXmlFile);

doc.getDocumentElement().normalize();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import java.util.List;
import java.util.Map;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.FactoryConfigurationError;

import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -140,13 +139,10 @@ private void buildInputs(String fileName) throws SubmissionConfigReaderException
String uri = "file:" + new File(fileName).getAbsolutePath();

try {
DocumentBuilderFactory factory = DocumentBuilderFactory
.newInstance();
factory.setValidating(false);
factory.setIgnoringComments(true);
factory.setIgnoringElementContentWhitespace(true);

DocumentBuilder db = factory.newDocumentBuilder();
// This document builder factory will *not* disable external
// entities as they can be useful in managing large forms, but
// it will restrict them to the config dir containing submission definitions
DocumentBuilder db = XMLUtils.getTrustedDocumentBuilder(configDir);
Document doc = db.parse(uri);
doNodes(doc);
} catch (FactoryConfigurationError fe) {
Expand Down
Loading
Loading