Repository: shapesecurity/salvation
Pull request #157 (2016-11-08 06:59:49)

fixes GH-155: Allow "*" to match scheme of protected resource

shekyan: Addressed comments. 

Patch:
diff --git a/src/main/java/com/shapesecurity/salvation/Constants.java b/src/main/java/com/shapesecurity/salvation/Constants.java
index 371c04e..051a468 100644
--- a/src/main/java/com/shapesecurity/salvation/Constants.java
+++ b/src/main/java/com/shapesecurity/salvation/Constants.java
@@ -4,6 +4,7 @@ import java.util.regex.Pattern;
 
 @SuppressWarnings("MalformedRegex") public class Constants {
     public static final String schemePart = "[a-zA-Z][a-zA-Z0-9+\\-.]*";
+    public static final Pattern schemePattern = Pattern.compile("^(?<scheme>" + Constants.schemePart + ":)");
     public static final Pattern referrerTokenPattern = Pattern
         .compile("^(?:no-referrer|no-referrer-when-downgrade|origin" + "|origin-when-cross-origin|unsafe-url)$",
             Pattern.CASE_INSENSITIVE);
diff --git a/src/main/java/com/shapesecurity/salvation/data/GUID.java b/src/main/java/com/shapesecurity/salvation/data/GUID.java
index 22b92bb..1efa8a3 100644
--- a/src/main/java/com/shapesecurity/salvation/data/GUID.java
+++ b/src/main/java/com/shapesecurity/salvation/data/GUID.java
@@ -1,6 +1,10 @@
 package com.shapesecurity.salvation.data;
 
+import com.shapesecurity.salvation.Constants;
+
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import java.util.regex.Matcher;
 
 public class GUID extends Origin {
     @Nonnull public String value;
@@ -15,6 +19,14 @@ public class GUID extends Origin {
         return ((GUID) other).value.equalsIgnoreCase(this.value);
     }
 
+    @Nullable public String scheme() {
+        Matcher matcher = Constants.schemePattern.matcher(this.value);
+        if (matcher.find()) {
+            return matcher.group("scheme");
+        }
+        return null;
+    }
+
     @Override public int hashCode() {
         return this.value.toLowerCase().hashCode();
     }
diff --git a/src/main/java/com/shapesecurity/salvation/data/URI.java b/src/main/java/com/shapesecurity/salvation/data/URI.java
index 037b94f..ab71f05 100644
--- a/src/main/java/com/shapesecurity/salvation/data/URI.java
+++ b/src/main/java/com/shapesecurity/salvation/data/URI.java
@@ -70,6 +70,12 @@ public class URI extends SchemeHostPortTriple implements DirectiveValue {
         return h;
     }
 
+    public boolean isNetworkScheme() {
+        return this.scheme != null && (this.scheme.equalsIgnoreCase("ftp") || this.scheme.equalsIgnoreCase("http") ||
+                this.scheme.equalsIgnoreCase("https") || this.scheme.equalsIgnoreCase("ws") ||
+                this.scheme.equalsIgnoreCase("wss"));
+    }
+
     @Nonnull @Override public String show() {
         return super.show() + this.path;
     }
diff --git a/src/main/java/com/shapesecurity/salvation/directiveValues/HostSource.java b/src/main/java/com/shapesecurity/salvation/directiveValues/HostSource.java
index c30d18e..a978533 100644
--- a/src/main/java/com/shapesecurity/salvation/directiveValues/HostSource.java
+++ b/src/main/java/com/shapesecurity/salvation/directiveValues/HostSource.java
@@ -59,40 +59,52 @@ public class HostSource implements SourceExpression, AncestorSource, MatchesSour
         return this.host.equals("*") && this.scheme == null && this.port == Constants.EMPTY_PORT && this.path == null;
     }
 
-    @Override public boolean matchesSource(@Nonnull Origin origin, @Nonnull URI source) {
-        if (!(origin instanceof SchemeHostPortTriple)) {
+    @Override public boolean matchesSource(@Nonnull Origin origin, @Nonnull URI resource) {
+        if (origin instanceof GUID) {
+            // wildcard matches a network scheme
+            return this.isWildcard() && resource.isNetworkScheme();
+        } else if (!(origin instanceof SchemeHostPortTriple)) {
             return false;
         }
         SchemeHostPortTriple shpOrigin = (SchemeHostPortTriple) origin;
         if (this.isWildcard()) {
-            return true;
+            return resource.isNetworkScheme() || shpOrigin.scheme.matches(resource.scheme);
         }
         boolean schemeMatches;
         if (this.scheme == null) {
-            schemeMatches = source.scheme.equalsIgnoreCase("http") ?
+            schemeMatches = resource.scheme.equalsIgnoreCase("http") ?
                 shpOrigin.scheme.equalsIgnoreCase("http") || shpOrigin.scheme.equalsIgnoreCase("https") :
-                source.scheme.equalsIgnoreCase(shpOrigin.scheme);
+                resource.scheme.equalsIgnoreCase(shpOrigin.scheme);
         } else {
-            schemeMatches = this.scheme.equalsIgnoreCase(source.scheme);
+            schemeMatches = this.scheme.equalsIgnoreCase(resource.scheme);
         }
         boolean hostMatches = this.host.equals("*") || (this.host.startsWith("*.") ?
-            source.host.toLowerCase().endsWith(this.host.substring(1).toLowerCase()) :
-            this.host.equalsIgnoreCase(source.host));
-        boolean uriUsesDefaultPort = source.port == Constants.EMPTY_PORT
-            || SchemeHostPortTriple.defaultPortForProtocol(source.scheme) == source.port;
+            resource.host.toLowerCase().endsWith(this.host.substring(1).toLowerCase()) :
+            this.host.equalsIgnoreCase(resource.host));
+        boolean uriUsesDefaultPort = resource.port == Constants.EMPTY_PORT
+            || SchemeHostPortTriple.defaultPortForProtocol(resource.scheme) == resource.port;
         boolean thisUsesDefaultPort = this.scheme != null && (this.port == Constants.EMPTY_PORT
             || SchemeHostPortTriple.defaultPortForProtocol(this.scheme) == this.port);
         boolean portMatches = this.port == Constants.WILDCARD_PORT || (this.port == Constants.EMPTY_PORT ?
             uriUsesDefaultPort :
-            (source.port == Constants.EMPTY_PORT ? thisUsesDefaultPort : this.port == source.port));
+            (resource.port == Constants.EMPTY_PORT ? thisUsesDefaultPort : this.port == resource.port));
         boolean pathMatches = this.path == null || (this.path.endsWith("/") ?
-            source.path.startsWith(this.path) :
-            this.path.equals(source.path));
+            resource.path.startsWith(this.path) :
+            this.path.equals(resource.path));
         return schemeMatches && hostMatches && portMatches && pathMatches;
     }
 
-    @Override public boolean matchesSource(@Nonnull Origin origin, @Nonnull GUID source) {
-        return false;
+    @Override public boolean matchesSource(@Nonnull Origin origin, @Nonnull GUID resource) {
+        String originScheme = null;
+        if (origin instanceof GUID) {
+            originScheme = ((GUID)origin).scheme();
+        }
+        String resourceScheme = resource.scheme();
+        if (origin instanceof GUID && originScheme != null && resourceScheme != null) {
+            return originScheme.equalsIgnoreCase(resourceScheme);
+        } else {
+            return false;
+        }
     }
 
     public boolean matchesOnlyOrigin(@Nonnull SchemeHostPortTriple origin) {
diff --git a/src/main/java/com/shapesecurity/salvation/directiveValues/KeywordSource.java b/src/main/java/com/shapesecurity/salvation/directiveValues/KeywordSource.java
index 6f02b50..be53c58 100644
--- a/src/main/java/com/shapesecurity/salvation/directiveValues/KeywordSource.java
+++ b/src/main/java/com/shapesecurity/salvation/directiveValues/KeywordSource.java
@@ -20,12 +20,12 @@ public class KeywordSource implements SourceExpression, AncestorSource, MatchesS
         this.value = value;
     }
 
-    @Override public boolean matchesSource(@Nonnull Origin origin, @Nonnull URI source) {
-        return this == Self && origin.equals(source);
+    @Override public boolean matchesSource(@Nonnull Origin origin, @Nonnull URI resource) {
+        return this == Self && origin.equals(resource);
     }
 
-    @Override public boolean matchesSource(@Nonnull Origin origin, @Nonnull GUID source) {
-        return this == Self && origin.equals(source);
+    @Override public boolean matchesSource(@Nonnull Origin origin, @Nonnull GUID resource) {
+        return this == Self && origin.equals(resource);
     }
 
     @Nonnull @Override public String show() {
diff --git a/src/main/java/com/shapesecurity/salvation/directiveValues/SchemeSource.java b/src/main/java/com/shapesecurity/salvation/directiveValues/SchemeSource.java
index 4d41c27..8e6809f 100644
--- a/src/main/java/com/shapesecurity/salvation/directiveValues/SchemeSource.java
+++ b/src/main/java/com/shapesecurity/salvation/directiveValues/SchemeSource.java
@@ -3,6 +3,7 @@ package com.shapesecurity.salvation.directiveValues;
 
 import com.shapesecurity.salvation.data.GUID;
 import com.shapesecurity.salvation.data.Origin;
+import com.shapesecurity.salvation.data.SchemeHostPortTriple;
 import com.shapesecurity.salvation.data.URI;
 import com.shapesecurity.salvation.interfaces.MatchesSource;
 
@@ -16,18 +17,18 @@ public class SchemeSource implements SourceExpression, AncestorSource, MatchesSo
         this.value = value;
     }
 
-    @Override public boolean matchesSource(@Nonnull Origin origin, @Nonnull URI source) {
-        return this.value.matches(source.scheme);
+    @Override public boolean matchesSource(@Nonnull Origin origin, @Nonnull URI resource) {
+        SchemeHostPortTriple shpOrigin = (SchemeHostPortTriple) origin;
+        return this.value.matches(resource.scheme);
     }
 
-    @Override public boolean matchesSource(@Nonnull Origin origin, @Nonnull GUID source) {
-        return source.value.toLowerCase().startsWith(this.value.toLowerCase() + ":");
+    @Override public boolean matchesSource(@Nonnull Origin origin, @Nonnull GUID resource) {
+        SchemeHostPortTriple shpOrigin = (SchemeHostPortTriple) origin;
+        String resourceString = resource.value.toLowerCase();
+        return resourceString.startsWith(this.value.toLowerCase() + ":") ||
+                resourceString.startsWith(shpOrigin.scheme + ":");
     }
 
-    public boolean matchesProtectedScheme() {
-        return this.value.equalsIgnoreCase("about") || this.value.equalsIgnoreCase("blob") ||
-            this.value.equalsIgnoreCase("data") || this.value.equalsIgnoreCase("filesystem");
-    }
     // Note: WebSocket schemes are not networks schemes but CSP spec decided to treat them as equivalent to http/https
     public boolean matchesNetworkScheme() {
         return this.value.equalsIgnoreCase("ftp") || this.value.equalsIgnoreCase("http") ||
diff --git a/src/main/java/com/shapesecurity/salvation/directives/AncestorSourceListDirective.java b/src/main/java/com/shapesecurity/salvation/directives/AncestorSourceListDirective.java
index 88fc9c0..2b40c6a 100644
--- a/src/main/java/com/shapesecurity/salvation/directives/AncestorSourceListDirective.java
+++ b/src/main/java/com/shapesecurity/salvation/directives/AncestorSourceListDirective.java
@@ -14,13 +14,13 @@ abstract class AncestorSourceListDirective extends Directive<AncestorSource> imp
         super(name, ancestorSources);
     }
 
-    public boolean matchesSource(@Nonnull Origin origin, @Nonnull URI source) {
+    public boolean matchesSource(@Nonnull Origin origin, @Nonnull URI resource) {
         return this.values().filter(x -> x instanceof MatchesSource)
-            .anyMatch(x -> ((MatchesSource) x).matchesSource(origin, source));
+            .anyMatch(x -> ((MatchesSource) x).matchesSource(origin, resource));
     }
 
-    public boolean matchesSource(@Nonnull Origin origin, @Nonnull GUID source) {
+    public boolean matchesSource(@Nonnull Origin origin, @Nonnull GUID resource) {
         return this.values().filter(x -> x instanceof MatchesSource)
-            .anyMatch(x -> ((MatchesSource) x).matchesSource(origin, source));
+            .anyMatch(x -> ((MatchesSource) x).matchesSource(origin, resource));
     }
 }
diff --git a/src/main/java/com/shapesecurity/salvation/directives/SourceListDirective.java b/src/main/java/com/shapesecurity/salvation/directives/SourceListDirective.java
index 8c68ea6..a57b470 100644
--- a/src/main/java/com/shapesecurity/salvation/directives/SourceListDirective.java
+++ b/src/main/java/com/shapesecurity/salvation/directives/SourceListDirective.java
@@ -21,14 +21,14 @@ public abstract class SourceListDirective extends Directive<SourceExpression>
             .anyMatch(x -> ((MatchesHash) x).matchesHash(algorithm, hashValue));
     }
 
-    public boolean matchesSource(@Nonnull Origin origin, @Nonnull URI source) {
+    public boolean matchesSource(@Nonnull Origin origin, @Nonnull URI resource) {
         return this.values().filter(x -> x instanceof MatchesSource)
-            .anyMatch(x -> ((MatchesSource) x).matchesSource(origin, source));
+            .anyMatch(x -> ((MatchesSource) x).matchesSource(origin, resource));
     }
 
-    public boolean matchesSource(@Nonnull Origin origin, @Nonnull GUID source) {
+    public boolean matchesSource(@Nonnull Origin origin, @Nonnull GUID resource) {
         return this.values().filter(x -> x instanceof MatchesSource)
-            .anyMatch(x -> ((MatchesSource) x).matchesSource(origin, source));
+            .anyMatch(x -> ((MatchesSource) x).matchesSource(origin, resource));
     }
 
     public boolean matchesNonce(@Nonnull String nonce) {
diff --git a/src/main/java/com/shapesecurity/salvation/interfaces/MatchesSource.java b/src/main/java/com/shapesecurity/salvation/interfaces/MatchesSource.java
index f47c7d0..fd0dc38 100644
--- a/src/main/java/com/shapesecurity/salvation/interfaces/MatchesSource.java
+++ b/src/main/java/com/shapesecurity/salvation/interfaces/MatchesSource.java
@@ -7,7 +7,7 @@ import com.shapesecurity.salvation.data.URI;
 import javax.annotation.Nonnull;
 
 public interface MatchesSource {
-    boolean matchesSource(@Nonnull Origin origin, @Nonnull URI source);
+    boolean matchesSource(@Nonnull Origin origin, @Nonnull URI resource);
 
-    boolean matchesSource(@Nonnull Origin origin, @Nonnull GUID source);
+    boolean matchesSource(@Nonnull Origin origin, @Nonnull GUID resource);
 }


Test patch:
diff --git a/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java b/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
index df47ae2..425d520 100644
--- a/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
+++ b/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
@@ -6,10 +6,23 @@ import com.shapesecurity.salvation.data.Policy;
 import com.shapesecurity.salvation.data.URI;
 import com.shapesecurity.salvation.directiveValues.HashSource;
 import com.shapesecurity.salvation.directiveValues.MediaType;
-import com.shapesecurity.salvation.directives.*;
+import com.shapesecurity.salvation.directives.ChildSrcDirective;
+import com.shapesecurity.salvation.directives.ConnectSrcDirective;
+import com.shapesecurity.salvation.directives.DefaultSrcDirective;
+import com.shapesecurity.salvation.directives.DirectiveValue;
+import com.shapesecurity.salvation.directives.FontSrcDirective;
+import com.shapesecurity.salvation.directives.ImgSrcDirective;
+import com.shapesecurity.salvation.directives.MediaSrcDirective;
+import com.shapesecurity.salvation.directives.ObjectSrcDirective;
+import com.shapesecurity.salvation.directives.ReportUriDirective;
+import com.shapesecurity.salvation.directives.ScriptSrcDirective;
+import com.shapesecurity.salvation.directives.StyleSrcDirective;
 import org.junit.Test;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 
 public class PolicyQueryingTest extends CSPTest {
 
@@ -388,6 +401,47 @@ public class PolicyQueryingTest extends CSPTest {
         assertFalse(p.allowsScriptFromSource(new GUID("data:")));
         assertFalse(p.allowsScriptFromSource(new GUID("custom.scheme:")));
 
+        p = Parser.parse("script-src *", "applewebdata://example.com");
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("https://example.com")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com:81")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("ftp://example.com")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("ftp://example.com:80")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com/path")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com/PATH")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("ws://example.com/PATH")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("wss://example.com/PATH")));
+        assertFalse(p.allowsScriptFromSource(new GUID("data:")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("applewebdata://resource")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("somethingelse://resource")));
+
+        p = Parser.parse("script-src *", "file://resource");
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("https://example.com")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com:81")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("ftp://example.com")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("ftp://example.com:80")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com/path")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com/PATH")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("ws://example.com/PATH")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("wss://example.com/PATH")));
+        assertFalse(p.allowsScriptFromSource(new GUID("data:")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("file://anotherresource")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("applewebdata://resource")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("somethingelse://resource")));
+
+        p = Parser.parse("script-src *", new GUID("data:text/html,%3Ch1%3EHello%2C%20World!%3C%2Fh1%3E"));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("https://example.com")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com:81")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("ftp://example.com")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("ftp://example.com:80")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com/path")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com/PATH")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("ws://example.com/PATH")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("wss://example.com/PATH")));
+        assertTrue(p.allowsScriptFromSource(new GUID("data:")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("somethingelse://resource")));
 
         p = Parser.parse("script-src http://*", "http://example.com");
         assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com")));