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
36b21fb
Avoid inline display of HTML/JS bitstreams. Add JS to list of known …
tdonohue Jun 6, 2024
8e73ec6
For additional security, ensure "unknown" formats are always download…
tdonohue Jun 10, 2024
d5448b7
To avoid misconfiguration, hardcode HTML, XML, RDF, JS to download on…
tdonohue Jun 10, 2024
9712baf
Potential fix for code scanning alert no. 30: Resolving XML external …
tdonohue Apr 29, 2025
d3fb769
Cannot disable DTDs with PubMed, so instead disallow external entitie…
tdonohue Apr 29, 2025
ef84b4d
Potential fix for code scanning alert no. 3549: Arbitrary file access…
tdonohue May 9, 2025
c95ff73
Safe and consistent XML entity handling in parsers
kshepherd Jul 2, 2025
0e76845
EPO and PubMed only need to allow for DOCTYPEs. All other XML securi…
tdonohue Jul 2, 2025
3de9b03
Allow trusted XML builder to enforce base path for entities
kshepherd Jul 10, 2025
9a19364
Enforce path traversal check on import subdir (pre-processing)
kshepherd Jul 13, 2025
c9914eb
Re-add file separator to normalized SAF item path
kshepherd Jul 14, 2025
ff6f884
Remove unused imports
kshepherd Jul 14, 2025
f84e598
Fix missing XMLUtils imports
kshepherd Jul 14, 2025
eecc1f6
fix: prevent path traversal in SAF import
MMilosz Jul 4, 2025
141f995
Enforce bitstream path to be within (fs) bitstore base on get
kshepherd Jul 9, 2025
7bf466c
return existing File constructed and validated for bitstream
kshepherd Jul 9, 2025
b1d4136
Fix line length in DSBitstore log
kshepherd Jul 14, 2025
48e89e7
ORE aggregated resource URI validation
kshepherd May 16, 2026
3dd149f
Velocity and template safety for Email and LDN messages
kshepherd May 5, 2026
983d2fa
Better null checking in allowed config props
kshepherd May 16, 2026
3da59cd
Access configurationService at runtime, not rely on class setup
kshepherd May 27, 2026
611535f
Remove strict mode Velocity engine configuration (allow nulls)
kshepherd May 27, 2026
b1c0db7
Filter requests for JSPs or traversal
kshepherd May 16, 2026
ffef5b9
Add additional logging to GlobalRequestSecurityFilter
kshepherd May 16, 2026
ad78148
Fix import order
kshepherd May 27, 2026
19c13f7
Update sitemap traversal test expectations
kshepherd May 27, 2026
93828ba
Backport GlobalRequestSecurityFilter for javax
kshepherd May 27, 2026
c411368
Add secure file access methods
kshepherd May 27, 2026
9e1fac6
Backport Curation I/O using secure file access
kshepherd May 27, 2026
d8c2c9b
Curation config support for allowed base paths
kshepherd May 27, 2026
c47fbc0
Move curation -r reporter param to CLI only
kshepherd May 26, 2026
4cdb7bc
Fix import order
kshepherd May 27, 2026
29e0002
Ignore CurationScriptIT -T taskFile tests, to rewrite w/ CLI
kshepherd May 27, 2026
8d856c3
Move taskfile -T option to CLI script config only
kshepherd May 27, 2026
20d3f81
Fix failing IT by increasing number of formats by one
milanmajchrak Jun 11, 2026
9371d35
Replace removed openjdk base images with eclipse-temurin
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
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ RUN mvn --no-transfer-progress package && \
mvn clean

# Step 2 - Run Ant Deploy
FROM openjdk:${JDK_VERSION}-slim as ant_build
FROM docker.io/eclipse-temurin:${JDK_VERSION} AS ant_build
ARG TARGET_DIR=dspace-installer
# COPY the /install directory from 'build' container to /dspace-src in this container
COPY --from=build /install /dspace-src
Expand Down
4 changes: 2 additions & 2 deletions Dockerfile.cli
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ RUN mvn --no-transfer-progress package && \
mvn clean

# Step 2 - Run Ant Deploy
FROM openjdk:${JDK_VERSION}-slim as ant_build
FROM docker.io/eclipse-temurin:${JDK_VERSION} AS ant_build
ARG TARGET_DIR=dspace-installer
# COPY the /install directory from 'build' container to /dspace-src in this container
COPY --from=build /install /dspace-src
Expand All @@ -45,7 +45,7 @@ RUN mkdir $ANT_HOME && \
RUN ant init_installation update_configs update_code

# Step 3 - Run jdk
FROM openjdk:${JDK_VERSION}
FROM docker.io/eclipse-temurin:${JDK_VERSION}
# NOTE: DSPACE_INSTALL must align with the "dspace.dir" default configuration.
ENV DSPACE_INSTALL=/dspace
# Copy the /dspace directory from 'ant_build' container to /dspace in this container
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile.test
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ RUN mvn --no-transfer-progress package -Pdspace-rest && \
mvn clean

# Step 2 - Run Ant Deploy
FROM openjdk:${JDK_VERSION}-slim as ant_build
FROM docker.io/eclipse-temurin:${JDK_VERSION} AS ant_build
ARG TARGET_DIR=dspace-installer
# COPY the /install directory from 'build' container to /dspace-src in this container
COPY --from=build /install /dspace-src
Expand Down
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
Loading
Loading