Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -11,13 +11,17 @@
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;
import java.util.Arrays;
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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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<Character> URLcharsSet = new HashSet<Character>();
URLcharsSet.addAll(Arrays.asList(lowalpha));
Expand All @@ -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<String> 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;
}
}

}
83 changes: 24 additions & 59 deletions dspace-api/src/main/java/org/dspace/core/Email.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.util.Date;
import java.util.Enumeration;
import java.util.List;
import java.util.Properties;
import java.util.stream.Collectors;
import javax.activation.DataHandler;
import javax.activation.DataSource;
Expand All @@ -45,12 +44,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;
Expand All @@ -73,17 +70,17 @@
* Apache Velocity</a>. They may contain VTL directives and property
* placeholders.
* <p>
* {@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.
* <p>
* The DSpace configuration properties are also available to templates as the
* array {@code config}, indexed by name. Example: {@code ${config.get('dspace.name')}}
* <p>
* 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)}.
* <p>
* 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
Expand Down Expand Up @@ -126,8 +123,8 @@
* </pre>
* <p>
* 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.
*
Expand Down Expand Up @@ -173,18 +170,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;
Expand All @@ -203,6 +188,13 @@ public Email() {
charset = null;
}

/**
* Get configuration service
*/
private static ConfigurationService getConfigurationService() {
return DSpaceServicesFactory.getInstance().getConfigurationService();
}

/**
* Add a recipient.
*
Expand All @@ -225,7 +217,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);
Expand Down Expand Up @@ -255,7 +247,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);
Expand Down Expand Up @@ -333,7 +326,7 @@ public void reset() {
* {@code mail.message.headers} then that name and its value will be added
* to the message's headers.
*
* <p>"subject" is treated specially: if {@link setSubject()} has not been
* <p>"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.
Expand All @@ -347,16 +340,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
Expand All @@ -371,11 +361,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();
Expand Down Expand Up @@ -672,31 +664,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);
}
}
}
Loading
Loading