From d4b7055498f4124132b9653a962814ed1decab53 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Sat, 16 May 2026 22:05:27 +0200 Subject: [PATCH 01/17] ORE aggregated resource URI validation (cherry picked from commit 7ac17f68432cff1e9463f05421fc8a29516000b8) --- .../crosswalk/OREIngestionCrosswalk.java | 84 +++++++++++++++++-- dspace/config/modules/oai.cfg | 7 ++ 2 files changed, 83 insertions(+), 8 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/content/crosswalk/OREIngestionCrosswalk.java b/dspace-api/src/main/java/org/dspace/content/crosswalk/OREIngestionCrosswalk.java index f756aae22577..9e890a6046fa 100644 --- a/dspace-api/src/main/java/org/dspace/content/crosswalk/OREIngestionCrosswalk.java +++ b/dspace-api/src/main/java/org/dspace/content/crosswalk/OREIngestionCrosswalk.java @@ -11,6 +11,8 @@ import java.io.IOException; import java.io.InputStream; import java.net.ConnectException; +import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import java.sql.SQLException; import java.text.NumberFormat; @@ -18,6 +20,8 @@ import java.util.Date; import java.util.HashSet; import java.util.List; +import java.util.Locale; +import java.util.Objects; import java.util.Set; import org.apache.logging.log4j.Logger; @@ -34,6 +38,8 @@ import org.dspace.content.service.ItemService; import org.dspace.core.Constants; import org.dspace.core.Context; +import org.dspace.services.ConfigurationService; +import org.dspace.services.factory.DSpaceServicesFactory; import org.jdom2.Attribute; import org.jdom2.Document; import org.jdom2.Element; @@ -76,6 +82,7 @@ public class OREIngestionCrosswalk .getBitstreamFormatService(); protected BundleService bundleService = ContentServiceFactory.getInstance().getBundleService(); protected ItemService itemService = ContentServiceFactory.getInstance().getItemService(); + protected ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); @Override @@ -173,9 +180,13 @@ public void ingest(Context context, DSpaceObject dso, Element root, boolean crea try { // Make sure the url string escapes all the oddball characters String processedURL = encodeForURL(href); - // Generate a requeset for the aggregated resource - ARurl = new URL(processedURL); - in = ARurl.openStream(); + if (validResourceUri(entryId, processedURL)) { + // Generate a request for the aggregated resource + ARurl = new URL(processedURL); + in = ARurl.openStream(); + } else { + throw new FileNotFoundException("Failed to validate " + processedURL); + } } catch (FileNotFoundException fe) { log.error("The provided URI failed to return a resource: " + href); } catch (ConnectException fe) { @@ -219,17 +230,17 @@ public void ingest(Context context, DSpaceObject dso, Element root, boolean crea * @param sourceString source unescaped string */ private String encodeForURL(String sourceString) { - Character lowalpha[] = {'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', + Character[] lowalpha = {'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z'}; - Character upalpha[] = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', + Character[] upalpha = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z'}; - Character digit[] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9'}; - Character mark[] = {'-', '_', '.', '!', '~', '*', '\'', '(', ')'}; + Character[] digit = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9'}; + Character[] mark = {'-', '_', '.', '!', '~', '*', '\'', '(', ')'}; // reserved - Character reserved[] = {';', '/', '?', ':', '@', '&', '=', '+', '$', ',', '%', '#'}; + Character[] reserved = {';', '/', '?', ':', '@', '&', '=', '+', '$', ',', '%', '#'}; Set URLcharsSet = new HashSet(); URLcharsSet.addAll(Arrays.asList(lowalpha)); @@ -251,4 +262,61 @@ private String encodeForURL(String sourceString) { return processedString.toString(); } + /** + * Validate a resource URI against the host and scheme of the remote OAI endpoint, or a configured + * list of allowed prefixes. + * This still implicitly "trusts" the remote OAI server, but will reject resource URIs with a totally + * different hostname to avoid downloading malicious resources from a compromised endpoint. + * Even if the URL prefix validation is disabled, schemes will still be enforced to http(s) so file:/// and + * other unwanted schemes cannot be used + * @param entryUrl the entryId of the parent ORE resource + * @param resourceUrl the resource URL of the aggregated ORE resource + * @return result of the validation + */ + private boolean validResourceUri(String entryUrl, String resourceUrl) { + try { + Set allowedSchemes = Set.of("http", "https"); + URI entryUri = new URI(entryUrl).normalize(); + URI resourceUri = new URI(resourceUrl).normalize(); + String scheme = resourceUri.getScheme(); + + if (scheme == null || + !allowedSchemes.contains(scheme.toLowerCase(Locale.ROOT))) { + log.warn("Illegal scheme requested for ORE resource: {}", resourceUri); + return false; + } + + if (configurationService.getBooleanProperty("oai.harvester.ore.file.validateUrlPrefix", false)) { + for (String allowedPrefix : configurationService + .getArrayProperty("oai.harvester.ore.file.allowedUrlPrefix")) { + URI allowedUri = new URI(allowedPrefix).normalize(); + // Return true on the first allowed prefix match + if (Objects.equals(resourceUri.getScheme(), allowedUri.getScheme()) + && Objects.equals(resourceUri.getHost().toLowerCase(Locale.ROOT), + allowedUri.getHost().toLowerCase(Locale.ROOT))) { + return true; + } + } + + // If no allowed prefixes were matched, we require scheme + host to match the remote OAI server + if (!Objects.equals(entryUri.getScheme(), resourceUri.getScheme())) { + log.warn("Illegal scheme requested for ORE resource: {}", resourceUri); + return false; + } + if (!Objects.equals( + entryUri.getHost().toLowerCase(Locale.ROOT), + resourceUri.getHost().toLowerCase(Locale.ROOT))) { + log.warn("Illegal host requested for ORE resource: {}", resourceUri); + return false; + } + } + + return true; + + } catch (URISyntaxException e) { + log.warn("Could not validate ORE resource URI: {}", resourceUrl); + return false; + } + } + } diff --git a/dspace/config/modules/oai.cfg b/dspace/config/modules/oai.cfg index ad30b609594c..a602f0539106 100644 --- a/dspace/config/modules/oai.cfg +++ b/dspace/config/modules/oai.cfg @@ -145,3 +145,10 @@ oai.harvester.unknownSchema = fail # when attempting to find the handle of harvested items. If there is a match with # this config parameter, a new handle will be minted instead. Default value: 123456789. #oai.harvester.rejectedHandlePrefix = 123456789, myTestHandle + +# If ingesting files with ORE, only files with URLs that match the base URL of the remote +# OAI endpoint's domain name are accepted, or a list of other URL prefixes defined below +#oai.harvester.ore.file.validateUrlPrefix = true +# Prefixes that are allowed globally (for any endpoint) are below +#oai.harvester.ore.file.allowedUrlPrefix = dspace.myinstitution.edu +#oai.harvester.ore.file.allowedUrlPrefix = files.myinstitution.edu From bd258ed56ae48d187a8de67794de149c660fb11b Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Tue, 5 May 2026 16:24:29 +0200 Subject: [PATCH 02/17] Velocity and template safety for Email and LDN messages * Safer Velocity configuration * New "message.templates.allowed-config" config * Remove "UnmodifiableConfiguration" in favour of a simple Map of whitelisted Config keys/values * Centralise Velocity config in core Utils * Small javadoc changes (cherry picked from commit b2d6141389f5652970b366325ed9deff21a86836) (cherry picked from commit 5b31db512f62bb530b71cb8fe85b1300f35e5601) --- .../src/main/java/org/dspace/core/Email.java | 83 ++++++------------- .../src/main/java/org/dspace/core/Utils.java | 64 ++++++++++++++ dspace/config/dspace.cfg | 12 +++ 3 files changed, 100 insertions(+), 59 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/core/Email.java b/dspace-api/src/main/java/org/dspace/core/Email.java index c157d16d4967..22bb3dc39630 100644 --- a/dspace-api/src/main/java/org/dspace/core/Email.java +++ b/dspace-api/src/main/java/org/dspace/core/Email.java @@ -22,7 +22,6 @@ import java.util.Date; import java.util.Enumeration; import java.util.List; -import java.util.Properties; import javax.activation.DataHandler; import javax.activation.DataSource; import javax.activation.FileDataSource; @@ -44,12 +43,10 @@ import org.apache.logging.log4j.Logger; import org.apache.velocity.Template; import org.apache.velocity.VelocityContext; -import org.apache.velocity.app.Velocity; import org.apache.velocity.app.VelocityEngine; import org.apache.velocity.exception.MethodInvocationException; import org.apache.velocity.exception.ParseErrorException; import org.apache.velocity.exception.ResourceNotFoundException; -import org.apache.velocity.runtime.resource.loader.StringResourceLoader; import org.apache.velocity.runtime.resource.util.StringResourceRepository; import org.dspace.services.ConfigurationService; import org.dspace.services.factory.DSpaceServicesFactory; @@ -72,7 +69,7 @@ * Apache Velocity. They may contain VTL directives and property * placeholders. *

- * {@link addArgument(string)} adds a property to the {@code params} array + * {@link #addArgument(Object)} adds a property to the {@code params} array * in the Velocity context, which can be used to replace placeholder tokens * in the message. These arguments are indexed by number in the order they were * added to the message. @@ -80,9 +77,9 @@ * The DSpace configuration properties are also available to templates as the * array {@code config}, indexed by name. Example: {@code ${config.get('dspace.name')}} *

- * Recipients and attachments may be added as needed. See {@link addRecipient}, - * {@link addAttachment(File, String)}, and - * {@link addAttachment(InputStream, String, String)}. + * Recipients and attachments may be added as needed. See {@link #addRecipient}, + * {@link #addAttachment(File, String)}, and + * {@link #addAttachment(InputStream, String, String)}. *

* Headers such as Subject may be supplied by the template, by defining them * using the VTL directive {@code #set()}. Only headers named in the DSpace @@ -125,8 +122,8 @@ * *

* There are two ways to load a message body. One can create an instance of - * {@link Email} and call {@link setContent} on it, passing the body as a String. Or - * one can use the static factory method {@link getEmail} to load a file by its + * {@link Email} and call {@link #setContent} on it, passing the body as a String. Or + * one can use the static factory method {@link #getEmail} to load a file by its * complete filesystem path. In either case the text will be loaded into a * Velocity template. * @@ -172,18 +169,6 @@ public class Email { /** Velocity template settings. */ private static final String RESOURCE_REPOSITORY_NAME = "Email"; - private static final Properties VELOCITY_PROPERTIES = new Properties(); - static { - VELOCITY_PROPERTIES.put(Velocity.RESOURCE_LOADERS, "string"); - VELOCITY_PROPERTIES.put("resource.loader.string.description", - "Velocity StringResource loader"); - VELOCITY_PROPERTIES.put("resource.loader.string.class", - StringResourceLoader.class.getName()); - VELOCITY_PROPERTIES.put("resource.loader.string.repository.name", - RESOURCE_REPOSITORY_NAME); - VELOCITY_PROPERTIES.put("resource.loader.string.repository.static", - "false"); - } /** Velocity template for a message body */ private Template template; @@ -202,6 +187,13 @@ public Email() { charset = null; } + /** + * Get configuration service + */ + private static ConfigurationService getConfigurationService() { + return DSpaceServicesFactory.getInstance().getConfigurationService(); + } + /** * Add a recipient. * @@ -224,7 +216,7 @@ public void setContent(String name, String content) { arguments.clear(); VelocityEngine templateEngine = new VelocityEngine(); - templateEngine.init(VELOCITY_PROPERTIES); + templateEngine.init(Utils.getSecureVelocityProperties(RESOURCE_REPOSITORY_NAME)); StringResourceRepository repo = (StringResourceRepository) templateEngine.getApplicationAttribute(RESOURCE_REPOSITORY_NAME); @@ -254,7 +246,8 @@ public void setReplyTo(String email) { /** * Fill out the next argument in the template. * - * @param arg the value for the next argument + * @param arg the value for the next argument. If {@code null}, + * a zero-length string is substituted. */ public void addArgument(Object arg) { arguments.add(arg); @@ -332,7 +325,7 @@ public void reset() { * {@code mail.message.headers} then that name and its value will be added * to the message's headers. * - *

"subject" is treated specially: if {@link setSubject()} has not been + *

"subject" is treated specially: if {@link #setSubject} has not been * called, the value of any "subject" property will be used as if setSubject * had been called with that value. Thus a template may define its subject, * but the caller may override it. @@ -346,16 +339,13 @@ public void send() throws MessagingException, IOException { throw new MessagingException("Email has no body"); } - ConfigurationService config - = DSpaceServicesFactory.getInstance().getConfigurationService(); - // Get the mail configuration properties - String from = config.getProperty("mail.from.address"); - boolean disabled = config.getBooleanProperty("mail.server.disabled", false); + String from = getConfigurationService().getProperty("mail.from.address"); + boolean disabled = getConfigurationService().getBooleanProperty("mail.server.disabled", false); // If no character set specified, attempt to retrieve a default if (charset == null) { - charset = config.getProperty("mail.charset"); + charset = getConfigurationService().getProperty("mail.charset"); } // Get session @@ -370,11 +360,13 @@ public void send() throws MessagingException, IOException { new InternetAddress(recipient)); } // Get headers defined by the template. - String[] templateHeaders = config.getArrayProperty("mail.message.headers"); + String[] templateHeaders = getConfigurationService().getArrayProperty("mail.message.headers"); // Format the mail message body VelocityContext vctx = new VelocityContext(); - vctx.put("config", new UnmodifiableConfigurationService(config)); + // Pass a restricted (via configuration) list of resolved Configuration keys and values, for + // template lookup + vctx.put("config", Utils.getAllowedTemplateConfig()); vctx.put("params", Collections.unmodifiableList(arguments)); StringWriter writer = new StringWriter(); @@ -661,31 +653,4 @@ public OutputStream getOutputStream() throws IOException { throw new IOException("Cannot write to this read-only resource"); } } - - /** - * Wrap ConfigurationService to prevent templates from modifying - * the configuration. - */ - public static class UnmodifiableConfigurationService { - private final ConfigurationService configurationService; - - /** - * Swallow an instance of ConfigurationService. - * - * @param cs the real instance, to be wrapped. - */ - public UnmodifiableConfigurationService(ConfigurationService cs) { - configurationService = cs; - } - - /** - * Look up a key in the actual ConfigurationService. - * - * @param key to be looked up in the DSpace configuration. - * @return whatever value ConfigurationService associates with {@code key}. - */ - public String get(String key) { - return configurationService.getProperty(key); - } - } } diff --git a/dspace-api/src/main/java/org/dspace/core/Utils.java b/dspace-api/src/main/java/org/dspace/core/Utils.java index b373288492f8..bddfda7d0267 100644 --- a/dspace-api/src/main/java/org/dspace/core/Utils.java +++ b/dspace-api/src/main/java/org/dspace/core/Utils.java @@ -30,16 +30,22 @@ import java.util.Collections; import java.util.Date; import java.util.GregorianCalendar; +import java.util.List; +import java.util.Map; +import java.util.Properties; import java.util.Random; import java.util.StringTokenizer; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import com.coverity.security.Escape; import org.apache.commons.lang3.StringUtils; import org.apache.commons.text.StringSubstitutor; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.velocity.app.Velocity; +import org.apache.velocity.runtime.resource.loader.StringResourceLoader; import org.dspace.services.ConfigurationService; import org.dspace.services.factory.DSpaceServicesFactory; import org.hibernate.Session; @@ -108,6 +114,14 @@ public final class Utils { private static final Calendar outCal = GregorianCalendar.getInstance(); + // Allowed configuration properties to pass to Velocity templates (Email, LDN) + private static final String[] DEFAULT_ALLOWED_TEMPLATE_CONFIGS = { + "dspace.name", "dspace.shortname", "dspace.ui.url", + "mail.helpdesk", "mail.message.helpdesk.telephone", "mail.admin", "mail.admin.name"}; + + private static final ConfigurationService configurationService = + DSpaceServicesFactory.getInstance().getConfigurationService(); + /** * Private constructor */ @@ -578,4 +592,54 @@ public static String fetchUUIDFromUrl(String urlString) { throw new IllegalArgumentException("Invalid URL or UUID format: " + e.getMessage(), e); } } + + /** + * Get a list of allowed DSpace configuration property keys that will be exposed to Velocity templates + * (used in Email and LDN messages) as a simple Map of strings. + * @return Map of strings representing resolved configuration properties + */ + public static Map getAllowedTemplateConfig() { + // Pass a restricted (via configuration) list of resolved Configuration keys and values, for + // template lookup + List allowedConfigurationKeys = List.of(configurationService.getArrayProperty( + "message.templates.allowed-config", DEFAULT_ALLOWED_TEMPLATE_CONFIGS)); + return allowedConfigurationKeys.stream() + .map(key -> Map.entry(key, configurationService.getProperty(key))) + .filter(entry -> entry.getValue() != null) + .collect(Collectors.toMap( + Map.Entry::getKey, + Map.Entry::getValue + )); + } + + /** + * Create and return a set of default, secure Velocity configuration properties. + * @see {@link Email} + * + * @param resourceRepositoryName the templating context e.g. "LDN", "Email" + * @returns secure Velocity configuration for use with templating + */ + public static Properties getSecureVelocityProperties(String resourceRepositoryName) { + Properties secureVelocityProperties = new Properties(); + // Basic Velocity configuration + secureVelocityProperties.setProperty(Velocity.RESOURCE_LOADERS, "string"); + secureVelocityProperties.setProperty("resource.loader.string.description", + "Velocity StringResource loader"); + secureVelocityProperties.setProperty("resource.loader.string.class", + StringResourceLoader.class.getName()); + secureVelocityProperties.setProperty("resource.loader.string.repository.name", + resourceRepositoryName); + secureVelocityProperties.setProperty("resource.loader.string.repository.static", + "false"); + // Set secure default introspection and class restriction handling in Velocity + secureVelocityProperties.setProperty("introspector.uberspect.class", + "org.apache.velocity.util.introspection.SecureUberspector"); + secureVelocityProperties.setProperty("introspector.restrict.classes", + "java.lang.Class,java.lang.Runtime,java.lang.System"); + secureVelocityProperties.setProperty( "introspector.restrict.packages", + "java.lang.reflect,java.io,java.nio"); + secureVelocityProperties.setProperty("runtime.strict_mode.enable", "true"); + + return secureVelocityProperties; + } } diff --git a/dspace/config/dspace.cfg b/dspace/config/dspace.cfg index fbb1f36f33a8..1b589f4b8ace 100644 --- a/dspace/config/dspace.cfg +++ b/dspace/config/dspace.cfg @@ -162,6 +162,7 @@ mail.from.address = dspace@vsb.cz # will use the above settings to create a Session. #mail.session.name = Session + # When feedback is submitted via the Feedback form, it is sent to this address # Currently limited to one recipient! # if this property is empty or commented out, feedback form is disabled @@ -228,6 +229,17 @@ mail.message.headers = charset # Helpdesk telephone. Not email, but should be with other contact info. Optional. #mail.message.helpdesk.telephone = +1 555 555 5555 +# Allowed configuration properties, to pass in a "config" map to email and LDN templates. +# This allows templates to easily access dynamic configuration properties, without +# exposing sensitive information to the templating engine +message.templates.allowed-config = dspace.name +message.templates.allowed-config = dspace.shortname +message.templates.allowed-config = dspace.ui.url +message.templates.allowed-config = mail.helpdesk +message.templates.allowed-config = mail.message.helpdesk.telephone +message.templates.allowed-config = mail.admin +message.templates.allowed-config = mail.admin.name + ##### Asset Storage (bitstreams / files) ###### # Moved to config/spring/api/bitstore.xml From f83bca9d2c735367ca68463be052f53b95a9ff29 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Sat, 16 May 2026 15:55:24 +0200 Subject: [PATCH 03/17] Better null checking in allowed config props (cherry picked from commit 6b665313cb48131ada04ae0840ff531b08b31dad) (cherry picked from commit 46a0dfb38197dfd9fa9970aa293e3a11a67c12b0) --- dspace-api/src/main/java/org/dspace/core/Utils.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/core/Utils.java b/dspace-api/src/main/java/org/dspace/core/Utils.java index bddfda7d0267..97d733efc187 100644 --- a/dspace-api/src/main/java/org/dspace/core/Utils.java +++ b/dspace-api/src/main/java/org/dspace/core/Utils.java @@ -32,6 +32,7 @@ import java.util.GregorianCalendar; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Properties; import java.util.Random; import java.util.StringTokenizer; @@ -604,8 +605,11 @@ public static Map getAllowedTemplateConfig() { List allowedConfigurationKeys = List.of(configurationService.getArrayProperty( "message.templates.allowed-config", DEFAULT_ALLOWED_TEMPLATE_CONFIGS)); return allowedConfigurationKeys.stream() - .map(key -> Map.entry(key, configurationService.getProperty(key))) - .filter(entry -> entry.getValue() != null) + .map(key -> { + String value = configurationService.getProperty(key); + return value != null ? Map.entry(key, value) : null; + }) + .filter(Objects::nonNull) .collect(Collectors.toMap( Map.Entry::getKey, Map.Entry::getValue From bfcc16e0ca7c968e64d30e652f73c4e1881f1614 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 27 May 2026 09:57:21 +0200 Subject: [PATCH 04/17] Access configurationService at runtime, not rely on class setup (cherry picked from commit 5803819ba65e7211a4f49318d0d3bbf2246e21c1) (cherry picked from commit 4be430f4f0d404b88ad87454af8aeafefae9c042) --- dspace-api/src/main/java/org/dspace/core/Utils.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/core/Utils.java b/dspace-api/src/main/java/org/dspace/core/Utils.java index 97d733efc187..8f850b38f7e4 100644 --- a/dspace-api/src/main/java/org/dspace/core/Utils.java +++ b/dspace-api/src/main/java/org/dspace/core/Utils.java @@ -120,9 +120,6 @@ public final class Utils { "dspace.name", "dspace.shortname", "dspace.ui.url", "mail.helpdesk", "mail.message.helpdesk.telephone", "mail.admin", "mail.admin.name"}; - private static final ConfigurationService configurationService = - DSpaceServicesFactory.getInstance().getConfigurationService(); - /** * Private constructor */ @@ -602,6 +599,8 @@ public static String fetchUUIDFromUrl(String urlString) { public static Map getAllowedTemplateConfig() { // Pass a restricted (via configuration) list of resolved Configuration keys and values, for // template lookup + ConfigurationService configurationService = + DSpaceServicesFactory.getInstance().getConfigurationService(); List allowedConfigurationKeys = List.of(configurationService.getArrayProperty( "message.templates.allowed-config", DEFAULT_ALLOWED_TEMPLATE_CONFIGS)); return allowedConfigurationKeys.stream() From 717cbf280379d4f692c5f77987227af750b112f8 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 27 May 2026 15:39:41 +0200 Subject: [PATCH 05/17] Remove strict mode Velocity engine configuration (allow nulls) (cherry picked from commit 655fc62874e9e4e5cf95ef2ff1e05b908484fc9f) --- dspace-api/src/main/java/org/dspace/core/Utils.java | 6 +++++- dspace/config/dspace.cfg | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/dspace-api/src/main/java/org/dspace/core/Utils.java b/dspace-api/src/main/java/org/dspace/core/Utils.java index 8f850b38f7e4..c49c15b451e2 100644 --- a/dspace-api/src/main/java/org/dspace/core/Utils.java +++ b/dspace-api/src/main/java/org/dspace/core/Utils.java @@ -641,7 +641,11 @@ public static Properties getSecureVelocityProperties(String resourceRepositoryNa "java.lang.Class,java.lang.Runtime,java.lang.System"); secureVelocityProperties.setProperty( "introspector.restrict.packages", "java.lang.reflect,java.io,java.nio"); - secureVelocityProperties.setProperty("runtime.strict_mode.enable", "true"); + // Set strict mode if configured (default: false, as we've always treated null values as blanks) + if (DSpaceServicesFactory.getInstance().getConfigurationService() + .getBooleanProperty("message.templates.strict_mode", false)) { + secureVelocityProperties.setProperty("runtime.strict_mode.enable", "true"); + } return secureVelocityProperties; } diff --git a/dspace/config/dspace.cfg b/dspace/config/dspace.cfg index 1b589f4b8ace..f63b1b95c87b 100644 --- a/dspace/config/dspace.cfg +++ b/dspace/config/dspace.cfg @@ -240,6 +240,11 @@ message.templates.allowed-config = mail.message.helpdesk.telephone message.templates.allowed-config = mail.admin message.templates.allowed-config = mail.admin.name +# Whether to run Velocity in strict mode (null parameter values in templates for LDN or Email will result +# in an Exception instead of a blank string) +# Default: false (this can introduce unwanted side-effects if e.g. a submitter eperson is deleted for a workflow task) +#message.templates.strict_mode = false + ##### Asset Storage (bitstreams / files) ###### # Moved to config/spring/api/bitstore.xml From 5d803534415538d433a58c331aa11716e8447f15 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Sat, 16 May 2026 20:54:09 +0200 Subject: [PATCH 06/17] Filter requests for JSPs or traversal (cherry picked from commit cf9be8554d3597e2c80958cd62336b40b79ba19d) (cherry picked from commit dc3e4553641bdd91bac344d0402ad796b80a75f9) --- .../security/GlobalRequestSecurityFilter.java | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java new file mode 100644 index 000000000000..53d6f7065333 --- /dev/null +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java @@ -0,0 +1,134 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.rest.security; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.springframework.core.Ordered; +import org.springframework.core.annotation.Order; +import org.springframework.stereotype.Component; +import org.springframework.web.filter.OncePerRequestFilter; + +import java.io.IOException; +import java.net.URI; +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; +import java.util.Locale; + +/** + * Global filter acting on all requests (not just /api/) to provide some additional hardening + * against common attacks or RCE, if a malicious payload was somehow written to a directory + * executable by the servlet container. + * The decoding and normalisation is designed to be tolerant of malformed URLs or broken clients, etc. + * so that this additional security filter does not introduce false positives or unintended side effects. + * + * @author Kim Shepherd + */ +@Component +@Order(Ordered.HIGHEST_PRECEDENCE) +public class GlobalRequestSecurityFilter extends OncePerRequestFilter { + + @Override + protected void doFilterInternal( + HttpServletRequest request, + HttpServletResponse response, + FilterChain filterChain + ) throws ServletException, IOException { + String normalizedPath = normaliseUrl(request.getRequestURI()); + // Return 403 forbidden if JSP execution or URL traversal is attempted + if (isTraversalAttempt(normalizedPath) || isJspExecutionAttempt(normalizedPath)) { + response.sendError(HttpServletResponse.SC_FORBIDDEN); + return; + } + filterChain.doFilter(request, response); + } + + /** + * Normalise the URI similarly to Tomcat, for testing how it will be interpreted + * @param rawUrl the unvalidated URL string + * @return a decoded, normalise URL + */ + private String normaliseUrl(String rawUrl) throws IOException { + if (rawUrl == null || rawUrl.isBlank()) { + throw new IOException("Empty URL"); + } + String url = rawUrl.split("\\?")[0]; + // Strip ;jspsession=... and so on + int semicolon = url.indexOf(';'); + if (semicolon >= 0) { + url = url.substring(0, semicolon); + } + url = decodeUrl(url); + if (url == null || url.isBlank()) { + throw new IOException("Decoded URL path is empty"); + } + url = normaliseUrlPath(url); + if (url == null || url.isBlank()) { + throw new IOException("Normalised URL path is empty"); + } + return url.toLowerCase(Locale.ROOT); + } + + /** + * Decode URL, falling back to original URL if it's malformed or undecodable + * @param url the encoded / unvalidated URL + * @return decoded URL or the original URL on error + */ + private String decodeUrl(String url) { + try { + return URLDecoder.decode(url, StandardCharsets.UTF_8); + } catch (IllegalArgumentException ex) { + // if we can't decode it, just return raw string + return url; + } + } + + /** + * Normalise the URL path and ensure it ends in a / + * @param url the URL path to normalise + * @return normalised path or the original parameter on error + */ + private String normaliseUrlPath(String url) { + try { + if (!url.startsWith("/")) { + url = "/" + url; + } + return new URI(url).normalize().getPath(); + } catch (Exception e) { + // if we can't use or normalise the path, just return the raw string + return url; + } + } + + /** + * Detect traversal after normalisation + * @param url the URL path to validate + * @return true if this looks like a traversal attempt + */ + private boolean isTraversalAttempt(String url) { + return url.contains("../") + || url.contains("/..") + || url.contains("%2e%2e") + || url.contains(".."); + } + + /** + * Block JSP execution attempts + * @param url the URL path to validate + */ + private boolean isJspExecutionAttempt(String url) { + return url.endsWith(".jsp") + || url.endsWith(".jspx") + || url.contains(".jsp/") + || url.contains(".jspx/") + || url.contains(".jsp\0") + || url.contains(".jspx\0"); + } +} \ No newline at end of file From 23496fb5073e7c487aa561fe54d2eb59cb1ee5f1 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Sat, 16 May 2026 22:18:27 +0200 Subject: [PATCH 07/17] Add additional logging to GlobalRequestSecurityFilter (cherry picked from commit 295a046fba14502619b3e3d96a7f6abdc9a4a5fc) (cherry picked from commit 0b1deae3fe94f55fb3dcda8dd6d6ba436b417f38) --- .../app/rest/security/GlobalRequestSecurityFilter.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java index 53d6f7065333..13740a53c7a4 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java @@ -43,7 +43,13 @@ protected void doFilterInternal( ) throws ServletException, IOException { String normalizedPath = normaliseUrl(request.getRequestURI()); // Return 403 forbidden if JSP execution or URL traversal is attempted - if (isTraversalAttempt(normalizedPath) || isJspExecutionAttempt(normalizedPath)) { + if (isTraversalAttempt(normalizedPath)) { + logger.warn("Path traversal attempt detected. Skipping request: " + request.getRequestURI()); + response.sendError(HttpServletResponse.SC_FORBIDDEN); + return; + } + if (isJspExecutionAttempt(normalizedPath)) { + logger.warn("JSP execution attempt detected. Skipping request: " + request.getRequestURI()); response.sendError(HttpServletResponse.SC_FORBIDDEN); return; } From 052a14408783e8f23a947eb65ae6fc7b336ca41c Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 27 May 2026 11:00:17 +0200 Subject: [PATCH 08/17] Fix import order (cherry picked from commit e2e6a796fd8d19de18a80b735e20a62d29c3c5cd) (cherry picked from commit 2e400771353f2a6e50bfb0067e2c58ca602e04c9) --- .../rest/security/GlobalRequestSecurityFilter.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java index 13740a53c7a4..78e433a8fad9 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java @@ -7,6 +7,12 @@ */ package org.dspace.app.rest.security; +import java.io.IOException; +import java.net.URI; +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; +import java.util.Locale; + import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; @@ -16,12 +22,6 @@ import org.springframework.stereotype.Component; import org.springframework.web.filter.OncePerRequestFilter; -import java.io.IOException; -import java.net.URI; -import java.net.URLDecoder; -import java.nio.charset.StandardCharsets; -import java.util.Locale; - /** * Global filter acting on all requests (not just /api/) to provide some additional hardening * against common attacks or RCE, if a malicious payload was somehow written to a directory @@ -137,4 +137,4 @@ private boolean isJspExecutionAttempt(String url) { || url.contains(".jsp\0") || url.contains(".jspx\0"); } -} \ No newline at end of file +} From 9ba49fbcd251b5f4067f722941b6445495b877aa Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 27 May 2026 13:38:55 +0200 Subject: [PATCH 09/17] Update sitemap traversal test expectations (cherry picked from commit 56ae2871eaba764900e4d9e23685a9472f485069) (cherry picked from commit 1a3dfd7c1a341ce9c04ee1aef0bea9f45c24dcbc) --- .../org/dspace/app/rest/SitemapRestControllerIT.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/SitemapRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/SitemapRestControllerIT.java index 04d22718e846..084b8272bd09 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/SitemapRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/SitemapRestControllerIT.java @@ -14,8 +14,6 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; -import javax.servlet.ServletException; - import org.dspace.app.rest.test.AbstractControllerIntegrationTest; import org.dspace.authorize.service.ResourcePolicyService; import org.dspace.builder.CollectionBuilder; @@ -131,18 +129,20 @@ public void testSitemap_notValidSiteMapFile() throws Exception { .andExpect(status().isNotFound()); } - @Test(expected = ServletException.class) + @Test public void testSitemap_fileSystemTraversal_dspaceCfg() throws Exception { //** WHEN ** //We attempt to use endpoint for malicious file system traversal - getClient().perform(get("/" + SITEMAPS_ENDPOINT + "/%2e%2e/config/dspace.cfg")); + getClient().perform(get("/" + SITEMAPS_ENDPOINT + "/%2e%2e/config/dspace.cfg")) + .andExpect(status().isForbidden()); } - @Test(expected = ServletException.class) + @Test public void testSitemap_fileSystemTraversal_dspaceCfg2() throws Exception { //** WHEN ** //We attempt to use endpoint for malicious file system traversal - getClient().perform(get("/" + SITEMAPS_ENDPOINT + "/%2e%2e%2fconfig%2fdspace.cfg")); + getClient().perform(get("/" + SITEMAPS_ENDPOINT + "/%2e%2e%2fconfig%2fdspace.cfg")) + .andExpect(status().isForbidden()); } @Test From 01c2d24a149668b23a8a02126862194a5f274adc Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 27 May 2026 16:30:28 +0200 Subject: [PATCH 10/17] Backport GlobalRequestSecurityFilter for javax (cherry picked from commit 8a2eee9d4adcbb4d858252877df8b020426e0802) --- .../app/rest/security/GlobalRequestSecurityFilter.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java index 78e433a8fad9..983da703878b 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java @@ -13,10 +13,10 @@ import java.nio.charset.StandardCharsets; import java.util.Locale; -import jakarta.servlet.FilterChain; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; import org.springframework.stereotype.Component; From 263ad168a381c12594b479b768567aa4bf283011 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 27 May 2026 13:45:20 +0200 Subject: [PATCH 11/17] Add secure file access methods (cherry picked from commit 22bec4459def712f529ff41283a8c7c5bcd1889c) --- .../storage/secure/SecureFileAccess.java | 166 ++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 dspace-api/src/main/java/org/dspace/storage/secure/SecureFileAccess.java diff --git a/dspace-api/src/main/java/org/dspace/storage/secure/SecureFileAccess.java b/dspace-api/src/main/java/org/dspace/storage/secure/SecureFileAccess.java new file mode 100644 index 000000000000..45727d9ee5ae --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/storage/secure/SecureFileAccess.java @@ -0,0 +1,166 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.storage.secure; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; + +/** + * Decent I/O path validation - not perfect when symlinks are used and we are writing + * as 'toRealPath' check on the resolved path fails for new files + * + * @author Kim Shepherd + */ +public final class SecureFileAccess { + + private SecureFileAccess() {} + + /** + * Validate a given path against an allowed base path. Does not attempt to calculate "real path" + * before validation, as this breaks for new files which don't yet exist. This can make the resulting + * validation still vulnerable to symlink traversal in some cases + * @param file the unvalidated file, usually derived from user input or configuration + * This MUST be an absolute path, and the caller is expected to calculate it based on best + * context (e.g. configured base path, CWD, dspace.dir, and so on) + * @param allowedBasePaths list of allowed base paths for this use case as per system configuration + * @param purpose the name of the calling component / use case for logging and inspection + * @throws IOException on validation failure + */ + public static Path validatePathForWrite(String file, List allowedBasePaths, String purpose) + throws IOException { + Path filePath = Path.of(file); + if (!filePath.isAbsolute()) { + throw new IOException("Absolute path required for I/O (" + purpose + "): " + file); + } + for (String allowedBasePath : allowedBasePaths) { + Path basePath = Path.of(allowedBasePath) + .toRealPath() + .normalize(); + Path resolvedPath = basePath.resolve(file).normalize(); + if (resolvedPath.startsWith(basePath)) { + return resolvedPath; + } + } + + // If no valid path was resolved and returned by now + // we raise an exception and treat this as illegal access + throw new IOException("Illegal file path attempted for I/O (" + purpose + "): " + file); + } + + /** + * Validate a given path against an allowed base path. + * More secure than the 'write' variant because we can explicitly resolve links as well. + * + * @param file the unvalidated file, usually derived from user input or configuration + * This MUST be an absolute path, and the caller is expected to calculate it based on best + * context (e.g. configured base path, CWD, dspace.dir, and so on) + * @param allowedBasePaths the allowed base paths for this use case as per system configuration + * @param purpose the name of the calling component / use case for logging and inspection + * @throws IOException on validation failure + */ + public static Path validatePathForRead(String file, List allowedBasePaths, String purpose) + throws IOException { + Path filePath = Path.of(file); + if (!filePath.isAbsolute()) { + throw new IOException("Absolute path required for I/O (" + purpose + "): " + file); + } + for (String allowedBasePath : allowedBasePaths) { + Path basePath = Path.of(allowedBasePath) + .toRealPath() + .normalize(); + Path resolvedPath = basePath.resolve(file).toRealPath().normalize(); + if (resolvedPath.startsWith(basePath)) { + return resolvedPath; + } + } + // If no valid path was resolved and returned by now + // we raise an exception and treat this as illegal access + throw new IOException("Illegal file path attempted for I/O (" + purpose + "): " + file); + } + + /** + * Get a buffered reader after validating file path. + * @param unvalidatedFile the unvalidated file, usually derived from user input or configuration + * @param allowedBasePaths the allowed base paths for this use case as per system configuration + * @param purpose the name of the calling component / use case for logging and inspection + * @throws IOException on validation failure + */ + public static BufferedReader getBufferedReader(String unvalidatedFile, List allowedBasePaths, + String purpose, Charset charset) throws IOException { + if (charset == null) { + charset = StandardCharsets.UTF_8; + } + Path validatedFile = validatePathForRead(unvalidatedFile, allowedBasePaths, purpose); + return Files.newBufferedReader(validatedFile, charset); + } + + /** + * Get an input stream after validating file path. + * @param unvalidatedFile the unvalidated file, usually derived from user input or configuration + * @param allowedBasePaths the allowed base paths for this use case as per system configuration + * @param purpose the name of the calling component / use case for logging and inspection + * @throws IOException on validation failure + */ + public static InputStream getInputStream(String unvalidatedFile, List allowedBasePaths, String purpose) + throws IOException { + Path validatedFile = validatePathForRead(unvalidatedFile, allowedBasePaths, purpose); + return Files.newInputStream(validatedFile); + + } + + /** + * Get an output stream after validating file path. New files can't use toRealPath() for link calculation so + * there is a bit of a trade-off in allowing some symlink traversal to occur + * @param unvalidatedFile the unvalidated file, usually derived from user input or configuration + * @param allowedBasePaths the allowed base paths for this use case as per system configuration + * @param purpose the name of the calling component / use case for logging and inspection + * @throws IOException on validation failure + */ + public static OutputStream getOutputStream(String unvalidatedFile, List allowedBasePaths, String purpose) + throws IOException { + Path validatedFile = validatePathForWrite(unvalidatedFile, allowedBasePaths, purpose); + return Files.newOutputStream(validatedFile); + } + + /** + * Calculate an absolute path (if not already absolute) using current working dir as a root + * for relative file paths + * @param file the relative or absolute file given as input + * @return absolute path calculated from file and cwd + */ + public static String calculateAbsolutePathUsingCwd(String file) { + String filePath = file; + Path path = Path.of(filePath); + if (!path.isAbsolute()) { + filePath = Path.of("").toAbsolutePath().resolve(path).normalize().toString(); + } + return filePath; + } + + /** + * Calculate an absolute path (if not already absolute) using a given base dir as a root + * for relative file paths + * @param file the relative or absolute file given as input + * @return absolute path calculated from file and base dir + */ + public static String calculateAbsolutePathUsingBaseDir(String file, String baseDir) { + String filePath = file; + Path path = Path.of(filePath); + if (!path.isAbsolute()) { + filePath = Path.of(baseDir).toAbsolutePath().resolve(path).normalize().toString(); + } + return filePath; + } +} From 860a17e9debb87b6e651e31055aecdf76ae92045 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 27 May 2026 13:49:50 +0200 Subject: [PATCH 12/17] Backport Curation I/O using secure file access Removes some JDK >= 16 usage (cherry picked from commit 55905a2fc46b98194f92ec38e0fb8bafa7fee21a) --- .../main/java/org/dspace/curate/Curation.java | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/curate/Curation.java b/dspace-api/src/main/java/org/dspace/curate/Curation.java index b894dcd85f03..43decd4add72 100644 --- a/dspace-api/src/main/java/org/dspace/curate/Curation.java +++ b/dspace-api/src/main/java/org/dspace/curate/Curation.java @@ -10,15 +10,18 @@ import java.io.BufferedReader; import java.io.File; import java.io.FileNotFoundException; -import java.io.FileReader; import java.io.IOException; import java.io.OutputStream; import java.io.OutputStreamWriter; import java.io.PrintStream; import java.io.Writer; +import java.nio.charset.StandardCharsets; import java.sql.SQLException; +import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.UUID; @@ -38,6 +41,7 @@ import org.dspace.handle.service.HandleService; import org.dspace.scripts.DSpaceRunnable; import org.dspace.services.factory.DSpaceServicesFactory; +import org.dspace.storage.secure.SecureFileAccess; import org.dspace.utils.DSpace; /** @@ -112,8 +116,16 @@ private void handleCurationTask(Curator curator) throws IOException, SQLExceptio } else if (commandLine.hasOption('T')) { // load taskFile BufferedReader reader = null; + // in this case, Curation CLI expects to calculate the -T parameter from the user's current working dir + String taskFilePath = SecureFileAccess.calculateAbsolutePathUsingCwd(this.taskFile); try { - reader = new BufferedReader(new FileReader(this.taskFile)); + String dspaceDir = DSpaceServicesFactory.getInstance() + .getConfigurationService().getProperty("dspace.dir"); + List allowedTaskFileBasePath = new ArrayList<>( + Arrays.asList(DSpaceServicesFactory.getInstance().getConfigurationService() + .getArrayProperty("curate.taskfile.base", new String[]{dspaceDir}))); + reader = SecureFileAccess.getBufferedReader(taskFilePath, allowedTaskFileBasePath, + "curation-taskfile", StandardCharsets.UTF_8); while ((taskName = reader.readLine()) != null) { if (verbose) { super.handler.logInfo("Adding task: " + taskName); @@ -189,12 +201,25 @@ private void endScript(long timeRun) throws SQLException { private Curator initCurator() throws FileNotFoundException { Curator curator = new Curator(handler); OutputStream reporterStream; + String dspaceDir = DSpaceServicesFactory.getInstance() + .getConfigurationService().getProperty("dspace.dir"); + List allowedReporterBasePaths = new ArrayList<>(Arrays.asList(DSpaceServicesFactory.getInstance() + .getConfigurationService().getArrayProperty("curate.reporter.base", + new String[]{dspaceDir + File.separatorChar + "log"}))); if (null == this.reporter) { reporterStream = NullOutputStream.NULL_OUTPUT_STREAM; } else if ("-".equals(this.reporter)) { reporterStream = System.out; } else { - reporterStream = new PrintStream(this.reporter); + // Reporter param comes from CLI execution. Calculate abs path from user's current working dir + String reporterFilePath = SecureFileAccess.calculateAbsolutePathUsingCwd(this.reporter); + try { + reporterStream = new PrintStream( + SecureFileAccess.getOutputStream( + reporterFilePath, allowedReporterBasePaths, "curation-reporter")); + } catch (IOException e) { + throw new FileNotFoundException(e.getLocalizedMessage()); + } } Writer reportWriter = new OutputStreamWriter(reporterStream); curator.setReporter(reportWriter); From acde70376d940bda3bb6ce1b375a726c221801c9 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 27 May 2026 13:50:59 +0200 Subject: [PATCH 13/17] Curation config support for allowed base paths (cherry picked from commit 45022245be2fabb5ba26d50b335f1aa1f905a660) --- dspace/config/modules/curate.cfg | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/dspace/config/modules/curate.cfg b/dspace/config/modules/curate.cfg index 6e75738de543..dad40d615454 100644 --- a/dspace/config/modules/curate.cfg +++ b/dspace/config/modules/curate.cfg @@ -29,3 +29,15 @@ curate.taskqueue.dir = ${dspace.dir}/ctqueues # Maximum amount of redirects set to 0 for none and -1 for unlimited curate.checklinks.max-redirect = 0 + +# allowed base path(s) of curation task files +# it is recommended to restrict this path as much as possible +# so that the DSpace Processes framework may only load files as "tasks" +# from a trusted location. For multiple paths, repeat this configuration +# property for each trusted path +# Default: ${dspace.dir} +#curate.taskfile.base = ${dspace.dir} + +# allowed base path of reporter output. +# Default: ${dspace.dir}/log +#curate.reporter.base = ${dspace.dir}/log From 77bb8f4adfdc438b8cf7c6cf9009a44e5a10c869 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Tue, 26 May 2026 14:06:14 +0200 Subject: [PATCH 14/17] Move curation -r reporter param to CLI only (cherry picked from commit 277af8233261ec0f61d71a4ce0908341c30e5e89) --- .../java/org/dspace/curate/CurationCliScriptConfiguration.java | 3 +++ .../src/main/java/org/dspace/curate/CurationClientOptions.java | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/curate/CurationCliScriptConfiguration.java b/dspace-api/src/main/java/org/dspace/curate/CurationCliScriptConfiguration.java index eaa04f477829..07684f3643d0 100644 --- a/dspace-api/src/main/java/org/dspace/curate/CurationCliScriptConfiguration.java +++ b/dspace-api/src/main/java/org/dspace/curate/CurationCliScriptConfiguration.java @@ -20,6 +20,9 @@ public Options getOptions() { options = super.getOptions(); options.addOption("e", "eperson", true, "email address of curating eperson"); options.getOption("e").setRequired(true); + options.addOption("r", "reporter", true, + "relative or absolute path to the desired report file. Use '-' to report to console. If absent, no " + + "reporting"); return options; } } diff --git a/dspace-api/src/main/java/org/dspace/curate/CurationClientOptions.java b/dspace-api/src/main/java/org/dspace/curate/CurationClientOptions.java index 8ec0f14697c0..a8a3d358ce67 100644 --- a/dspace-api/src/main/java/org/dspace/curate/CurationClientOptions.java +++ b/dspace-api/src/main/java/org/dspace/curate/CurationClientOptions.java @@ -59,9 +59,6 @@ protected static Options constructOptions() { "Id (handle) of object to perform task on, or 'all' to perform on whole repository"); options.addOption("p", "parameter", true, "a task parameter 'NAME=VALUE'"); options.addOption("q", "queue", true, "name of task queue to process"); - options.addOption("r", "reporter", true, - "relative or absolute path to the desired report file. Use '-' to report to console. If absent, no " + - "reporting"); options.addOption("s", "scope", true, "transaction scope to impose: use 'object', 'curation', or 'open'. If absent, 'open' applies"); options.addOption("v", "verbose", false, "report activity to stdout"); From 04ce619b573d6f73327b01bff3f3fa84b1d85a44 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 27 May 2026 16:57:21 +0200 Subject: [PATCH 15/17] Fix import order (cherry picked from commit a7572212c135155fb8420a2bfc95869f1ba6959d) --- .../dspace/app/rest/security/GlobalRequestSecurityFilter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java index 983da703878b..a9f0eea8d728 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GlobalRequestSecurityFilter.java @@ -12,11 +12,11 @@ import java.net.URLDecoder; import java.nio.charset.StandardCharsets; import java.util.Locale; - import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; + import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; import org.springframework.stereotype.Component; From 8bd0ee4763bad88e1cd6f41a37fd4672bda3a460 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 27 May 2026 14:38:00 +0200 Subject: [PATCH 16/17] Ignore CurationScriptIT -T taskFile tests, to rewrite w/ CLI (cherry picked from commit 6437472b8277b9aa815dd71e14b499ba7515f87d) (cherry picked from commit 37cd6eb791d4f61bb54fedf899aa96f99504e38d) --- .../src/test/java/org/dspace/curate/CurationScriptIT.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dspace-server-webapp/src/test/java/org/dspace/curate/CurationScriptIT.java b/dspace-server-webapp/src/test/java/org/dspace/curate/CurationScriptIT.java index 8745613d7af6..347ed0935f05 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/curate/CurationScriptIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/curate/CurationScriptIT.java @@ -49,6 +49,7 @@ import org.dspace.scripts.configuration.ScriptConfiguration; import org.dspace.scripts.factory.ScriptServiceFactory; import org.dspace.scripts.service.ScriptService; +import org.junit.Ignore; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -214,6 +215,7 @@ public void curateScript_InvalidScope() throws Exception { .andExpect(status().isBadRequest()); } + @Ignore @Test public void curateScript_InvalidTaskFile() throws Exception { String token = getAuthToken(admin.getEmail(), password); @@ -286,6 +288,7 @@ public void curateScript_validRequest_Task() throws Exception { } } + @Ignore @Test public void curateScript_validRequest_TaskFile() throws Exception { context.turnOffAuthorisationSystem(); From e6e5d60bf2225114869646dac7847a553feab250 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 27 May 2026 06:43:22 +0200 Subject: [PATCH 17/17] Move taskfile -T option to CLI script config only (cherry picked from commit 00e4979a60fd69adbf4a7476926701ef59207ce7) (cherry picked from commit 27708ea6d70abe433f131cf3b875dfdc067d3c12) --- .../org/dspace/curate/CurationCliScriptConfiguration.java | 1 + .../main/java/org/dspace/curate/CurationClientOptions.java | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/curate/CurationCliScriptConfiguration.java b/dspace-api/src/main/java/org/dspace/curate/CurationCliScriptConfiguration.java index 07684f3643d0..925bd4f2d232 100644 --- a/dspace-api/src/main/java/org/dspace/curate/CurationCliScriptConfiguration.java +++ b/dspace-api/src/main/java/org/dspace/curate/CurationCliScriptConfiguration.java @@ -23,6 +23,7 @@ public Options getOptions() { options.addOption("r", "reporter", true, "relative or absolute path to the desired report file. Use '-' to report to console. If absent, no " + "reporting"); + options.addOption("T", "taskfile", true, "file containing curation task names"); return options; } } diff --git a/dspace-api/src/main/java/org/dspace/curate/CurationClientOptions.java b/dspace-api/src/main/java/org/dspace/curate/CurationClientOptions.java index a8a3d358ce67..03ad2f34b230 100644 --- a/dspace-api/src/main/java/org/dspace/curate/CurationClientOptions.java +++ b/dspace-api/src/main/java/org/dspace/curate/CurationClientOptions.java @@ -31,7 +31,8 @@ public enum CurationClientOptions { /** * This method resolves the CommandLine parameters to figure out which action the curation script should perform * - * @param commandLine The relevant CommandLine for the curation script + * @param commandLine The relevant CommandLine for the curation script. Note that -T is passed only + * from CurationCliScriptConfig and is not accessible from UI processes * @return The curation option to be ran, parsed from the CommandLine */ protected static CurationClientOptions getClientOption(CommandLine commandLine) { @@ -54,7 +55,6 @@ protected static Options constructOptions() { Options options = new Options(); options.addOption("t", "task", true, "curation task name; options: " + getTaskOptions()); - options.addOption("T", "taskfile", true, "file containing curation task names"); options.addOption("i", "id", true, "Id (handle) of object to perform task on, or 'all' to perform on whole repository"); options.addOption("p", "parameter", true, "a task parameter 'NAME=VALUE'");