Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
cca1465
Avoid inline display of HTML/JS bitstreams. Add JS to list of known …
tdonohue Jun 6, 2024
5c02b31
For additional security, ensure "unknown" formats are always download…
tdonohue Jun 10, 2024
a4889bd
To avoid misconfiguration, hardcode HTML, XML, RDF, JS to download on…
tdonohue Jun 10, 2024
f0a5c31
Potential fix for code scanning alert no. 30: Resolving XML external …
tdonohue Apr 29, 2025
9f1bbc1
Cannot disable DTDs with PubMed, so instead disallow external entitie…
tdonohue Apr 29, 2025
85f8649
Potential fix for code scanning alert no. 3549: Arbitrary file access…
tdonohue May 9, 2025
622dd30
Safe and consistent XML entity handling in parsers
kshepherd Jul 2, 2025
c39e684
EPO and PubMed only need to allow for DOCTYPEs. All other XML securi…
tdonohue Jul 2, 2025
de6bc7c
Allow trusted XML builder to enforce base path for entities
kshepherd Jul 10, 2025
135eec7
Enforce path traversal check on import subdir (pre-processing)
kshepherd Jul 13, 2025
f33bca8
Re-add file separator to normalized SAF item path
kshepherd Jul 14, 2025
0e01832
Remove unused imports
kshepherd Jul 14, 2025
1f63734
Fix missing XMLUtils imports
kshepherd Jul 14, 2025
ba7b074
fix: prevent path traversal in SAF import
MMilosz Jul 4, 2025
44dadef
Enforce bitstream path to be within (fs) bitstore base on get
kshepherd Jul 9, 2025
ecbeca2
return existing File constructed and validated for bitstream
kshepherd Jul 9, 2025
791f39e
Fix line length in DSBitstore log
kshepherd Jul 14, 2025
b9cc4ea
ORE aggregated resource URI validation
kshepherd May 16, 2026
ba19c9a
Velocity and template safety for Email and LDN messages
kshepherd May 5, 2026
46f2a64
Better null checking in allowed config props
kshepherd May 16, 2026
436a8c9
Access configurationService at runtime, not rely on class setup
kshepherd May 27, 2026
31d8730
Remove strict mode Velocity engine configuration (allow nulls)
kshepherd May 27, 2026
d7e9ffa
Filter requests for JSPs or traversal
kshepherd May 16, 2026
4c49ac6
Add additional logging to GlobalRequestSecurityFilter
kshepherd May 16, 2026
d7f199d
Fix import order
kshepherd May 27, 2026
4996f01
Update sitemap traversal test expectations
kshepherd May 27, 2026
0edc856
Backport GlobalRequestSecurityFilter for javax
kshepherd May 27, 2026
3f1516c
Add secure file access methods
kshepherd May 27, 2026
0189275
Backport Curation I/O using secure file access
kshepherd May 27, 2026
f6bb8ae
Curation config support for allowed base paths
kshepherd May 27, 2026
6a4ef82
Move curation -r reporter param to CLI only
kshepherd May 26, 2026
d17c7ca
Fix import order
kshepherd May 27, 2026
0e1f5d7
Ignore CurationScriptIT -T taskFile tests, to rewrite w/ CLI
kshepherd May 27, 2026
7a9b356
Move taskfile -T option to CLI script config only
kshepherd May 27, 2026
a4d52ad
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 @@ -192,6 +193,8 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea
@Autowired(required = true)
protected ClarinItemService clarinItemService;

protected DocumentBuilder builder;

protected String tempWorkDir;

protected boolean isTest = false;
Expand Down Expand Up @@ -788,15 +791,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 @@ -814,8 +824,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 @@ -1047,6 +1056,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 @@ -1247,6 +1284,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 @@ -1460,6 +1498,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 @@ -1932,9 +1971,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.DSpaceRunnable.StepResult;
Expand Down Expand Up @@ -314,7 +315,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 @@ -130,15 +129,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
Loading
Loading