Improve the security model

This patch is mostly based on the following Batik commits by S. Steiner:

bfbccb6ad2
996aa8897c
3f937cfea7
905f368b50
eada57c716
f9ae69233e
85b3457d99
aaa1dd3e6b

Although the packages that are whitelisted by default are different, as well as
the whitelisting API.

Signed-off-by: Simon Steiner <ssteiner@apache.org>
Signed-off-by: Carlos Amengual <carlosame644+github@gmail.com>

SECURITY.md

@@ -0,0 +1,43 @@
+# Security and Privacy Considerations
+
+EchoSVG handles images in the Scalable Vector Graphics (SVG) format for various
+purposes, however it is important to understand its limitations related to security
+and privacy.
+
+<br/>
+
+## Security
+
+SVG documents can be complex and can drive any rendering software to its limits.
+In general, if a SVG document can cause issues to a web browser that attempts to
+render it, that image will also cause problems to Batik.
+
+Unfortunately, this library can also be less secure than web browsers, especially
+in scripting security. To execute scripts, EchoSVG relies on the Mozilla Rhino
+javascript library, which is embedded via a feature called LiveConnect. [It is well
+known that it is almost impossible to secure a Rhino environment that uses
+LiveConnect](https://github.com/mozilla/rhino/discussions/1045), so users are
+advised against running untrusted scripts, or any trusted script that could somehow
+be exploited by a malicious actor.
+
+If you have any concern about script security, please do not enable scripting at
+all in EchoSVG.
+
+<br/>
+
+## Privacy
+
+In addition to potential issues with javascript, even a static SVG document could
+leak information if a malicious CSS style sheet is used.
+
+In short, this library should not be used to process any sort of untrusted content,
+and you are advised to set up strict security measures (like a strongly secured
+operating system container) if you need to do that.
+
+<br/>
+
+## Using EchoSVG safely
+
+If you are concerned about security and privacy, be sure to learn about the features
+of this library that have security implications, like the `KEY_ALLOW_EXTERNAL_RESOURCES`,
+`KEY_CONSTRAIN_SCRIPT_ORIGIN` or `KEY_ALLOWED_SCRIPT_TYPES` transcoding hints.

buildSrc/src/main/groovy/echosvg.java-conventions.gradle

@@ -99,7 +99,8 @@ tasks.register('copyJars', Copy) {
 	dependsOn tasks.build
 	dependsOn ':echosvg-all:jar'
 	include '**/*.jar'
-	excludes = ['IWasLoaded*.jar', 'JarCheckPermissions*.jar', 'echosvg-*-jmh.jar']
+	excludes = ['IWasLoaded*.jar', 'JarCheckPermissions*.jar',
+		'JarPolicyTest.jar', 'echosvg-*-jmh.jar']
 	from layout.buildDirectory.dir("libs")
 	into "${rootDir}/jar"
 }

echosvg-bridge/src/main/java/io/sf/carte/echosvg/bridge/DefaultExternalResourceSecurity.java

@@ -18,6 +18,9 @@
  */
 package io.sf.carte.echosvg.bridge;
 
+import java.net.URI;
+import java.net.URISyntaxException;
+
 import io.sf.carte.echosvg.util.ParsedURL;
 
 /**
@@ -73,23 +76,34 @@ public void checkLoadExternalResource() {
 	public DefaultExternalResourceSecurity(ParsedURL externalResourceURL, ParsedURL docURL) {
 		// Make sure that the archives comes from the same host
 		// as the document itself
+		if (DATA_PROTOCOL.equals(externalResourceURL.getProtocol())) {
+			return;
+		}
 		if (docURL == null) {
 			se = new SecurityException(
 					Messages.formatMessage(ERROR_CANNOT_ACCESS_DOCUMENT_URL, new Object[] { externalResourceURL }));
 		} else {
 			String docHost = docURL.getHost();
+			String externalResourceHost = externalResourceURL.getHost();
 
-			if (externalResourceURL == null) {
-				throw new NullPointerException();
+			String externalPath;
+			if (externalResourceHost == null
+					&& !DATA_PROTOCOL.equals(externalResourceURL.getProtocol())
+					&& (externalPath = externalResourceURL.getPath()) != null) {
+				try {
+					externalResourceHost = new URI(externalPath).getHost();
+				} catch (URISyntaxException e) {
+					throw new RuntimeException(e);
+				}
 			}
 
-			String externalResourceHost = externalResourceURL.getHost();
-
-			if ((docHost != externalResourceHost) && ((docHost == null) || (!docHost.equals(externalResourceHost)))) {
+			if (docHost != externalResourceHost
+					&& (docHost == null || !docHost.equals(externalResourceHost))) {
 
 				if (!DATA_PROTOCOL.equals(externalResourceURL.getProtocol())) {
-					se = new SecurityException(Messages.formatMessage(ERROR_EXTERNAL_RESOURCE_FROM_DIFFERENT_URL,
-							new Object[] { externalResourceURL }));
+					se = new SecurityException(
+							Messages.formatMessage(ERROR_EXTERNAL_RESOURCE_FROM_DIFFERENT_URL,
+									new Object[] { externalResourceURL }));
 				}
 
 			}

echosvg-bridge/src/main/java/io/sf/carte/echosvg/bridge/DefaultScriptSecurity.java

@@ -18,7 +18,11 @@
  */
 package io.sf.carte.echosvg.bridge;
 
+import java.net.URI;
+import java.net.URISyntaxException;
+
 import io.sf.carte.echosvg.util.ParsedURL;
+import io.sf.carte.echosvg.util.SVGConstants;
 
 /**
  * Default implementation for the <code>ScriptSecurity</code> interface. It
@@ -30,7 +34,9 @@
  * @version $Id$
  */
 public class DefaultScriptSecurity implements ScriptSecurity {
+
 	public static final String DATA_PROTOCOL = "data";
+
 	/**
 	 * Message when trying to load a script file and the Document does not have a
 	 * URL
@@ -72,18 +78,23 @@ public void checkLoadScript() {
 	public DefaultScriptSecurity(String scriptType, ParsedURL scriptURL, ParsedURL docURL) {
 		// Make sure that the archives comes from the same host
 		// as the document itself
-		if (docURL == null) {
+		if (docURL == null || SVGConstants.SVG_SCRIPT_TYPE_JAVA.equals(scriptType)) {
 			se = new SecurityException(
 					Messages.formatMessage(ERROR_CANNOT_ACCESS_DOCUMENT_URL, new Object[] { scriptURL }));
 		} else {
 			String docHost = docURL.getHost();
+			String scriptHost = scriptURL.getHost();
 
-			if (scriptURL == null) {
-				throw new NullPointerException();
+			String externalPath;
+			if (scriptHost == null && !DATA_PROTOCOL.equals(scriptURL.getProtocol())
+					&& (externalPath = scriptURL.getPath()) != null) {
+				try {
+					scriptHost = new URI(externalPath).getHost();
+				} catch (URISyntaxException e) {
+					throw new RuntimeException(e);
+				}
 			}
 
-			String scriptHost = scriptURL.getHost();
-
 			if ((docHost != scriptHost) && ((docHost == null) || (!docHost.equals(scriptHost)))) {
 				if (!docURL.equals(scriptURL)
 						&& (scriptURL == null || !DATA_PROTOCOL.equals(scriptURL.getProtocol()))) {

echosvg-bridge/src/main/java/io/sf/carte/echosvg/bridge/SVGImageElementBridge.java

@@ -175,17 +175,12 @@ protected GraphicsNode buildImageGraphicsNode(BridgeContext ctx, Element e) {
 			purl = new ParsedURL(baseURI, uriStr);
 		}
 
+		checkLoadExternalResource(ctx, e, purl);
+
 		return createImageGraphicsNode(ctx, e, purl);
 	}
 
-	protected GraphicsNode createImageGraphicsNode(BridgeContext ctx, Element e, ParsedURL purl) {
-		Rectangle2D bounds = getImageBounds(ctx, e);
-		if ((bounds.getWidth() == 0) || (bounds.getHeight() == 0)) {
-			ShapeNode sn = new ShapeNode();
-			sn.setShape(bounds);
-			return sn;
-		}
-
+	private void checkLoadExternalResource(BridgeContext ctx, Element e, ParsedURL purl) {
 		SVGDocument svgDoc = (SVGDocument) e.getOwnerDocument();
 		String docURL = svgDoc.getURL();
 		ParsedURL pDocURL = null;
@@ -199,6 +194,15 @@ protected GraphicsNode createImageGraphicsNode(BridgeContext ctx, Element e, Par
 		} catch (SecurityException secEx) {
 			throw new BridgeException(ctx, e, secEx, ERR_URI_UNSECURE, new Object[] { purl });
 		}
+	}
+
+	protected GraphicsNode createImageGraphicsNode(BridgeContext ctx, Element e, ParsedURL purl) {
+		Rectangle2D bounds = getImageBounds(ctx, e);
+		if ((bounds.getWidth() == 0) || (bounds.getHeight() == 0)) {
+			ShapeNode sn = new ShapeNode();
+			sn.setShape(bounds);
+			return sn;
+		}
 
 		DocumentLoader loader = ctx.getDocumentLoader();
 		ImageTagRegistry reg = ImageTagRegistry.getRegistry();

echosvg-bridge/src/main/java/io/sf/carte/echosvg/bridge/UserAgentAdapter.java

@@ -358,8 +358,9 @@ public void checkLoadScript(String scriptType, ParsedURL scriptURL, ParsedURL do
 	 * @param docURL      url for the document into which the resource was found.
 	 */
 	@Override
-	public ExternalResourceSecurity getExternalResourceSecurity(ParsedURL resourceURL, ParsedURL docURL) {
-		return new RelaxedExternalResourceSecurity(resourceURL, docURL);
+	public ExternalResourceSecurity getExternalResourceSecurity(ParsedURL resourceURL,
+			ParsedURL docURL) {
+		return new DefaultExternalResourceSecurity(resourceURL, docURL);
 	}
 
 	/**

echosvg-bridge/src/main/resources/io/sf/carte/echosvg/bridge/resources/Messages.properties

@@ -121,7 +121,7 @@ DefaultExternalResourceSecurity.error.cannot.access.document.url = \
 Could not access the current document URL when trying to load an external resource {0}. The external resource will not be loaded as it is not possible to verify it comes from the same location as the document.
 
 DefaultExternalResourceSecurity.error.external.resource.from.different.url = \
-The document references a external resource ({0}) which comes from different location than the document itself. This is not allowed for security reasons and that resource will not be loaded.
+The document references an external resource ({0}) which comes from different location than the document itself. This is not allowed for security reasons and that resource will not be loaded.
 
 NoLoadExternalResourceSecurity.error.no.external.resource.allowed = \
 The security settings do not allow any external resources to be referenced from the document

echosvg-script/src/main/java/io/sf/carte/echosvg/script/rhino/RhinoClassShutter.java

@@ -18,6 +18,14 @@
  */
 package io.sf.carte.echosvg.script.rhino;
 
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.Reader;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
+
 import org.mozilla.javascript.ClassShutter;
 
 /**
@@ -29,21 +37,112 @@
  */
 public class RhinoClassShutter implements ClassShutter {
 
-	/*
-	 * public RhinoClassShutter() { // I suspect that we might want to initialize
-	 * this // from a resource file. // test(); }
+	private static final List<Pattern> whitelist = new ArrayList<>();
+
+	static {
+		addToWhitelist("java.io.PrintStream");
+		addToWhitelist("java.lang.System");
+		addToWhitelist("java.net.URL");
+		addToWhitelist(".*Permission");
+		addToWhitelist("org.w3c.*");
+		addToWhitelist("io.sf.carte.echosvg.w3c.*");
+		addToWhitelist("io.sf.carte.echosvg.anim.*");
+		addToWhitelist("io.sf.carte.echosvg.dom.*");
+		addToWhitelist("io.sf.carte.echosvg.css.*");
+		addToWhitelist("io.sf.carte.echosvg.util.*");
+	}
+
+	public RhinoClassShutter() {
+		super();
+	}
+
+	/**
+	 * Add the given regular expression to the whitelist.
+	 * 
+	 * @param regex the regular expression.
+	 * @throws PatternSyntaxException - if the regular expression's syntax is
+	 *                                invalid
+	 */
+	public static void addToWhitelist(String regex) throws PatternSyntaxException {
+		if (regex == null || (regex = regex.trim()).length() == 0) {
+			return;
+		}
+
+		for (Pattern p : whitelist) {
+			if (regex.equals(p.pattern())) {
+				// Already here
+				return;
+			}
+		}
+
+		Pattern p = Pattern.compile(regex);
+		whitelist.add(p);
+	}
+
+	/**
+	 * Remove the given regular expression from the whitelist.
+	 * 
+	 * @param regex the regular expression.
+	 * @throws PatternSyntaxException - if the regular expression's syntax is
+	 *                                invalid
+	 */
+	public static void removeFromWhitelist(String regex) throws PatternSyntaxException {
+		if (regex == null || (regex = regex.trim()).length() == 0) {
+			return;
+		}
+		for (Pattern p : whitelist) {
+			if (regex.equals(p.pattern())) {
+				whitelist.remove(p);
+				break;
+			}
+		}
+	}
+
+	/**
+	 * Loads a whitelist from the given reader.
+	 * <p>
+	 * Loading a whitelist does not reset the list, and only applies the new entries
+	 * to it.
+	 * </p>
+	 * <ul>
+	 * <li>If an entry begins with a {@code #}, the line will be ignored.</li>
+	 * <li>If an entry starts with a {@code -}, the entry shall be removed.</li>
+	 * <li>If begins with a {@code +} or with a character different from {@code -}
+	 * and {@code #}, the entry shall be added.</li>
+	 * </ul>
+	 * <p>
+	 * Example:
+	 * </p>
+	 * 
+	 * <pre>
+	 * # Comment
+	 * +java.io.PrintWriter
+	 * +com.example.*
+	 * -java.lang.System
+	 * </pre>
 	 * 
-	 * public void test() { test("org.mozilla.javascript.Context");
-	 * test("org.mozilla.javascript");
-	 * test("io.sf.carte.echosvg.dom.SVGOMDocument");
-	 * test("io.sf.carte.echosvg.script.rhino.RhinoInterpreter");
-	 * test("io.sf.carte.echosvg.apps.svgbrowser.JSVGViewerFrame");
-	 * test("io.sf.carte.echosvg.bridge.BridgeContext");
-	 * test("io.sf.carte.echosvg.bridge.BaseScriptingEnvironment");
-	 * test("io.sf.carte.echosvg.bridge.ScriptingEnvironment"); } public void
-	 * test(String cls) { System.err.println("Test '" + cls + "': " +
-	 * visibleToScripts(cls)); }
+	 * @param reader the reader.
+	 * @throws IOException            - if an I/O problem occurs when reading the
+	 *                                list
+	 * @throws PatternSyntaxException - if a regular expression's syntax is invalid
 	 */
+	public static void loadWhitelist(Reader reader) throws IOException, PatternSyntaxException {
+		BufferedReader re = new BufferedReader(reader);
+		String s;
+		while ((s = re.readLine()) != null) {
+			int len = s.length();
+			if (len > 0) {
+				char c = s.charAt(0);
+				if (c == '-') {
+					removeFromWhitelist(s.substring(1, len));
+				} else if (c == '+') {
+					addToWhitelist(s.substring(1, len));
+				} else if (c != '#') {
+					addToWhitelist(s);
+				}
+			}
+		}
+	}
 
 	/**
 	 * Returns whether the given class is visible to scripts.
@@ -56,7 +155,7 @@ public boolean visibleToScripts(String fullClassName) {
 
 		if (fullClassName.startsWith("io.sf.carte.echosvg.")) {
 			// Just get package within this implementation.
-			String implPkg = fullClassName.substring(17);
+			String implPkg = fullClassName.substring(20);
 
 			// Don't let them mess with this implementation's script internals.
 			if (implPkg.startsWith("script"))
@@ -81,8 +180,10 @@ public boolean visibleToScripts(String fullClassName) {
 				if (implBridgeClass.startsWith("ScriptingEnvironment")) {
 					if (implBridgeClass.startsWith("$Window$", 20)) {
 						String c = implBridgeClass.substring(28);
-						if (c.equals("IntervalScriptTimerTask") || c.equals("IntervalRunnableTimerTask")
-								|| c.equals("TimeoutScriptTimerTask") || c.equals("TimeoutRunnableTimerTask")) {
+						if (c.equals("IntervalScriptTimerTask")
+								|| c.equals("IntervalRunnableTimerTask")
+								|| c.equals("TimeoutScriptTimerTask")
+								|| c.equals("TimeoutRunnableTimerTask")) {
 							return true;
 						}
 					}
@@ -94,6 +195,13 @@ public boolean visibleToScripts(String fullClassName) {
 			}
 		}
 
-		return true;
+		for (Pattern p : whitelist) {
+			if (p.matcher(fullClassName).matches()) {
+				return true;
+			}
+		}
+
+		return false;
 	}
+
 }