Repository: shapesecurity/salvation
Pull request #132 (2016-04-19 23:03:13)

fixes GH-129
Require explicit declaration of schemes other than network schemes in source-expressions that contain "*"


Patch:
diff --git a/src/main/java/com/shapesecurity/salvation/data/Policy.java b/src/main/java/com/shapesecurity/salvation/data/Policy.java
index 9dea583..37d8473 100644
--- a/src/main/java/com/shapesecurity/salvation/data/Policy.java
+++ b/src/main/java/com/shapesecurity/salvation/data/Policy.java
@@ -142,8 +142,8 @@ public class Policy implements Show {
                     Set<SourceExpression> newSources = sourceListDirective.values()
                         // * remove all other host sources in a source list that contains *
                         .filter(x -> !(x instanceof HostSource))
-                        // * remove schemes sources other than about:, data:, blob:, and filesystem: in source list that contains *
-                        .filter(x -> !(x instanceof SchemeSource) || ((SchemeSource) x).matchesProtectedScheme())
+                        // * remove network-schemes in source list that contains *
+                        .filter(x -> !(x instanceof SchemeSource) || !((SchemeSource) x).matchesNetworkScheme())
                         .collect(Collectors.toCollection(LinkedHashSet::new));
                     newSources.add(star.get());
                     this.directives.put(entry.getKey(), sourceListDirective.construct(newSources));
diff --git a/src/main/java/com/shapesecurity/salvation/directiveValues/SchemeSource.java b/src/main/java/com/shapesecurity/salvation/directiveValues/SchemeSource.java
index 44b7b0e..4d41c27 100644
--- a/src/main/java/com/shapesecurity/salvation/directiveValues/SchemeSource.java
+++ b/src/main/java/com/shapesecurity/salvation/directiveValues/SchemeSource.java
@@ -28,6 +28,12 @@ public class SchemeSource implements SourceExpression, AncestorSource, MatchesSo
         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") ||
+            this.value.equalsIgnoreCase("https") || this.value.equalsIgnoreCase("ws") ||
+            this.value.equalsIgnoreCase("wss");
+    }
 
     @Override public boolean equals(@Nullable Object other) {
         if (other == null || !(other instanceof SchemeSource))


Test patch:
diff --git a/src/test/java/com/shapesecurity/salvation/ParserTest.java b/src/test/java/com/shapesecurity/salvation/ParserTest.java
index b0f9c09..0c922a9 100644
--- a/src/test/java/com/shapesecurity/salvation/ParserTest.java
+++ b/src/test/java/com/shapesecurity/salvation/ParserTest.java
@@ -192,7 +192,10 @@ public class ParserTest extends CSPTest {
         assertEquals("optimisation", "", parseAndShow("script-src 'self' *"));
         assertEquals("optimisation", "script-src 'unsafe-inline'; style-src 'unsafe-inline'",
             parseAndShow("script-src 'unsafe-inline'; style-src 'unsafe-inline';"));
-
+        assertEquals("optimisation with network scheme", "", parseAndShow("script-src 'self' * ftp:"));
+        assertEquals("optimisation with other scheme", "script-src data: *", parseAndShow("script-src 'self' * data:"));
+        assertEquals("optimisation with other scheme", "script-src custom: *", parseAndShow("script-src 'self' * custom:"));
+        assertEquals("optimisation with mixed schemes", "script-src custom: blob: *", parseAndShow("script-src 'self' * custom: ftp: blob:"));
 
     }
 
diff --git a/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java b/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
index a55b75a..37cb1bb 100644
--- a/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
+++ b/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
@@ -304,6 +304,7 @@ public class PolicyQueryingTest extends CSPTest {
         assertTrue(p.allowsFrameAncestor(new GUID("ABOUT:")));
         assertFalse(p.allowsFrameAncestor(new GUID("blob:")));
         assertFalse(p.allowsFrameAncestor(new GUID("BLOB:")));
+        assertFalse(p.allowsFrameAncestor(new GUID("custom.scheme:")));
 
         p = Parser.parse("script-src *.example.com DATA: BLOB:; frame-ancestors DATA: ABOUT:", "http://example.com");
         assertTrue(p.allowsScriptFromSource(new GUID("data:")));
@@ -318,6 +319,15 @@ public class PolicyQueryingTest extends CSPTest {
         assertTrue(p.allowsFrameAncestor(new GUID("ABOUT:")));
         assertFalse(p.allowsFrameAncestor(new GUID("blob:")));
         assertFalse(p.allowsFrameAncestor(new GUID("BLOB:")));
+        assertFalse(p.allowsFrameAncestor(new GUID("custom.scheme:")));
+
+        p = Parser.parse("script-src *.example.com custom-scheme:; frame-ancestors custom.scheme2:", "http://example.com");
+        assertFalse(p.allowsScriptFromSource(new GUID("custom.scheme:")));
+        assertTrue(p.allowsScriptFromSource(new GUID("custom-scheme:")));
+        assertFalse(p.allowsFrameAncestor(new GUID("BLOB:")));
+        assertFalse(p.allowsFrameAncestor(new GUID("custom-scheme:")));
+        assertTrue(p.allowsFrameAncestor(new GUID("custom.scheme2:")));
+
 
     }
 
@@ -332,7 +342,11 @@ public class PolicyQueryingTest extends CSPTest {
         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:")));
+        assertFalse(p.allowsScriptFromSource(new GUID("custom.scheme:")));
+
 
         p = Parser.parse("script-src http://*", "http://example.com");
         assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com")));
@@ -342,7 +356,10 @@ public class PolicyQueryingTest extends CSPTest {
         assertFalse(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")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("ws://example.com/PATH")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("wss://example.com/PATH")));
         assertFalse(p.allowsScriptFromSource(new GUID("data:")));
+        assertFalse(p.allowsScriptFromSource(new GUID("custom.scheme:")));
 
         p = Parser.parse("style-src *:80", "http://example.com");
         assertTrue(p.allowsStyleFromSource(URI.parse("http://example.com")));
@@ -352,7 +369,10 @@ public class PolicyQueryingTest extends CSPTest {
         assertFalse(p.allowsStyleFromSource(URI.parse("ftp://example.com:80")));
         assertTrue(p.allowsStyleFromSource(URI.parse("http://example.com/path")));
         assertTrue(p.allowsStyleFromSource(URI.parse("http://example.com/PATH")));
+        assertFalse(p.allowsStyleFromSource(URI.parse("ws://example.com/PATH")));
+        assertFalse(p.allowsStyleFromSource(URI.parse("wss://example.com/PATH")));
         assertFalse(p.allowsStyleFromSource(new GUID("data:")));
+        assertFalse(p.allowsStyleFromSource(new GUID("custom.scheme:")));
 
         p = Parser.parse("style-src *:80", "https://example.com");
         assertTrue(p.allowsStyleFromSource(URI.parse("http://example.com")));
@@ -361,7 +381,10 @@ public class PolicyQueryingTest extends CSPTest {
         assertFalse(p.allowsStyleFromSource(URI.parse("ftp://example.com")));
         assertFalse(p.allowsStyleFromSource(URI.parse("ftp://example.com:80")));
         assertTrue(p.allowsStyleFromSource(URI.parse("http://example.com/path")));
+        assertFalse(p.allowsStyleFromSource(URI.parse("ws://example.com/PATH")));
+        assertFalse(p.allowsStyleFromSource(URI.parse("wss://example.com/PATH")));
         assertFalse(p.allowsStyleFromSource(new GUID("data:")));
+        assertFalse(p.allowsStyleFromSource(new GUID("custom.scheme:")));
 
         p = Parser.parse("style-src *:80", "ftp://example.com");
         assertFalse(p.allowsStyleFromSource(URI.parse("http://example.com")));
@@ -370,7 +393,10 @@ public class PolicyQueryingTest extends CSPTest {
         assertFalse(p.allowsStyleFromSource(URI.parse("ftp://example.com")));
         assertTrue(p.allowsStyleFromSource(URI.parse("ftp://example.com:80")));
         assertFalse(p.allowsStyleFromSource(URI.parse("http://example.com/path")));
+        assertFalse(p.allowsStyleFromSource(URI.parse("ws://example.com/PATH")));
+        assertFalse(p.allowsStyleFromSource(URI.parse("wss://example.com/PATH")));
         assertFalse(p.allowsStyleFromSource(new GUID("data:")));
+        assertFalse(p.allowsStyleFromSource(new GUID("custom.scheme:")));
 
         p = Parser.parse("img-src ftp://*", "http://example.com");
         assertFalse(p.allowsImgFromSource(URI.parse("http://example.com")));
@@ -379,7 +405,10 @@ public class PolicyQueryingTest extends CSPTest {
         assertTrue(p.allowsImgFromSource(URI.parse("ftp://example.com")));
         assertFalse(p.allowsImgFromSource(URI.parse("ftp://example.com:80")));
         assertFalse(p.allowsImgFromSource(URI.parse("http://example.com/path")));
+        assertFalse(p.allowsImgFromSource(URI.parse("ws://example.com/PATH")));
+        assertFalse(p.allowsImgFromSource(URI.parse("wss://example.com/PATH")));
         assertFalse(p.allowsImgFromSource(new GUID("data:")));
+        assertFalse(p.allowsImgFromSource(new GUID("custom.scheme:")));
 
         p = Parser.parse("style-src *:*", "http://example.com");
         assertTrue(p.allowsStyleFromSource(URI.parse("http://example.com")));
@@ -388,7 +417,10 @@ public class PolicyQueryingTest extends CSPTest {
         assertFalse(p.allowsStyleFromSource(URI.parse("ftp://example.com")));
         assertFalse(p.allowsStyleFromSource(URI.parse("ftp://example.com:80")));
         assertTrue(p.allowsStyleFromSource(URI.parse("http://example.com/path")));
+        assertFalse(p.allowsStyleFromSource(URI.parse("ws://example.com/PATH")));
+        assertFalse(p.allowsStyleFromSource(URI.parse("wss://example.com/PATH")));
         assertFalse(p.allowsStyleFromSource(new GUID("data:")));
+        assertFalse(p.allowsStyleFromSource(new GUID("custom.scheme:")));
 
         p = Parser.parse("style-src http://*:*", "http://example.com");
         assertTrue(p.allowsStyleFromSource(URI.parse("http://example.com")));
@@ -397,7 +429,10 @@ public class PolicyQueryingTest extends CSPTest {
         assertFalse(p.allowsStyleFromSource(URI.parse("ftp://example.com")));
         assertFalse(p.allowsStyleFromSource(URI.parse("ftp://example.com:80")));
         assertTrue(p.allowsStyleFromSource(URI.parse("http://example.com/path")));
+        assertFalse(p.allowsStyleFromSource(URI.parse("ws://example.com/PATH")));
+        assertFalse(p.allowsStyleFromSource(URI.parse("wss://example.com/PATH")));
         assertFalse(p.allowsStyleFromSource(new GUID("data:")));
+        assertFalse(p.allowsStyleFromSource(new GUID("custom.scheme:")));
 
         p = Parser.parse("style-src ftp://*:*", "http://example.com");
         assertFalse(p.allowsStyleFromSource(URI.parse("http://example.com")));
@@ -406,7 +441,10 @@ public class PolicyQueryingTest extends CSPTest {
         assertTrue(p.allowsStyleFromSource(URI.parse("ftp://example.com")));
         assertTrue(p.allowsStyleFromSource(URI.parse("ftp://example.com:80")));
         assertFalse(p.allowsStyleFromSource(URI.parse("http://example.com/path")));
+        assertFalse(p.allowsStyleFromSource(URI.parse("ws://example.com/PATH")));
+        assertFalse(p.allowsStyleFromSource(URI.parse("wss://example.com/PATH")));
         assertFalse(p.allowsStyleFromSource(new GUID("data:")));
+        assertFalse(p.allowsStyleFromSource(new GUID("custom.scheme:")));
 
         p = Parser.parse("img-src */path", "http://example.com");
         assertFalse(p.allowsImgFromSource(URI.parse("http://example.com")));
@@ -415,7 +453,10 @@ public class PolicyQueryingTest extends CSPTest {
         assertFalse(p.allowsImgFromSource(URI.parse("ftp://example.com")));
         assertFalse(p.allowsImgFromSource(URI.parse("ftp://example.com:80")));
         assertTrue(p.allowsImgFromSource(URI.parse("http://example.com/path")));
+        assertFalse(p.allowsImgFromSource(URI.parse("ws://example.com/PATH")));
+        assertFalse(p.allowsImgFromSource(URI.parse("wss://example.com/PATH")));
         assertFalse(p.allowsImgFromSource(new GUID("data:")));
+        assertFalse(p.allowsImgFromSource(new GUID("custom.scheme:")));
 
         p = Parser.parse("script-src *.example.com", "http://example.com");
         assertTrue(p.allowsScriptFromSource(URI.parse("http://a.b.example.com/c/d")));
@@ -423,7 +464,10 @@ public class PolicyQueryingTest extends CSPTest {
         assertTrue(p.allowsScriptFromSource(URI.parse("http://www.example.com")));
         assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com")));
         assertFalse(p.allowsScriptFromSource(URI.parse("http://com")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("ws://example.com/PATH")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("wss://example.com/PATH")));
         assertFalse(p.allowsScriptFromSource(new GUID("data:")));
+        assertFalse(p.allowsScriptFromSource(new GUID("custom.scheme:")));
     }
 
 }