Repository: shapesecurity/salvation
Pull request #198 (2018-03-29 05:07:12)

Fix handling missing default-src
Previously, when answering questions about policy, we incorrectly assumed missing default-src is restrictive.


Patch:
diff --git a/src/main/java/com/shapesecurity/salvation/Parser.java b/src/main/java/com/shapesecurity/salvation/Parser.java
index 00b438b..be52449 100644
--- a/src/main/java/com/shapesecurity/salvation/Parser.java
+++ b/src/main/java/com/shapesecurity/salvation/Parser.java
@@ -47,7 +47,6 @@ import com.shapesecurity.salvation.directives.NavigateToDirective;
 import com.shapesecurity.salvation.directives.ObjectSrcDirective;
 import com.shapesecurity.salvation.directives.PluginTypesDirective;
 import com.shapesecurity.salvation.directives.PrefetchSrcDirective;
-
 import com.shapesecurity.salvation.directives.ReferrerDirective;
 import com.shapesecurity.salvation.directives.ReportToDirective;
 import com.shapesecurity.salvation.directives.ReportUriDirective;
diff --git a/src/main/java/com/shapesecurity/salvation/data/Policy.java b/src/main/java/com/shapesecurity/salvation/data/Policy.java
index 37a5425..66adb66 100644
--- a/src/main/java/com/shapesecurity/salvation/data/Policy.java
+++ b/src/main/java/com/shapesecurity/salvation/data/Policy.java
@@ -395,7 +395,7 @@ public class Policy implements Show {
             return false;
         DefaultSrcDirective defaultSrcDirective = this.getDirectiveByType(DefaultSrcDirective.class);
         if (defaultSrcDirective == null) {
-            return false;
+            return true;
         }
         return defaultSrcDirective.matchesHash(algorithm, hashValue);
     }
@@ -405,7 +405,7 @@ public class Policy implements Show {
             return true;
         DefaultSrcDirective defaultSrcDirective = this.getDirectiveByType(DefaultSrcDirective.class);
         if (defaultSrcDirective == null) {
-            return false;
+            return true;
         }
         return defaultSrcDirective.matchesHash(algorithm, hashValue);
     }
@@ -415,7 +415,7 @@ public class Policy implements Show {
             return true;
         DefaultSrcDirective defaultSrcDirective = this.getDirectiveByType(DefaultSrcDirective.class);
         if (defaultSrcDirective == null) {
-            return false;
+            return true;
         }
         return defaultSrcDirective.matchesNonce(nonce);
     }
@@ -423,7 +423,7 @@ public class Policy implements Show {
     private boolean defaultsAllowSource(@Nonnull URI source) {
         DefaultSrcDirective defaultSrcDirective = this.getDirectiveByType(DefaultSrcDirective.class);
         if (defaultSrcDirective == null) {
-            return false;
+            return true;
         }
         return defaultSrcDirective.matchesSource(this.origin, source);
     }
@@ -431,7 +431,7 @@ public class Policy implements Show {
     private boolean defaultsAllowSource(@Nonnull GUID source) {
         DefaultSrcDirective defaultSrcDirective = this.getDirectiveByType(DefaultSrcDirective.class);
         if (defaultSrcDirective == null) {
-            return false;
+            return true;
         }
         return defaultSrcDirective.matchesSource(this.origin, source);
     }


Test patch:
diff --git a/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java b/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
index 85828d0..a3c98f6 100644
--- a/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
+++ b/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
@@ -1248,13 +1248,13 @@ public class PolicyQueryingTest extends CSPTest {
         Policy p;
 
         p = Parser.parse("", "http://example.com");
-        assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com")));
-        assertFalse(p.allowsScriptFromSource(URI.parse("wss://example.com")));
-        assertFalse(p.allowsScriptWithNonce(new Base64Value("1234")));
-        assertFalse(p.allowsScriptWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value(
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("wss://example.com")));
+        assertTrue(p.allowsScriptWithNonce(new Base64Value("1234")));
+        assertTrue(p.allowsScriptWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value(
             "vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg==")));
-        assertFalse(p.allowsScriptFromSource(new GUID("custom.scheme:")));
-        assertFalse(p.allowsScriptFromSource(new GUID("data:")));
+        assertTrue(p.allowsScriptFromSource(new GUID("custom.scheme:")));
+        assertTrue(p.allowsScriptFromSource(new GUID("data:")));
     }
 
     @Test public void testHasSomeEffect() {
@@ -1288,8 +1288,8 @@ public class PolicyQueryingTest extends CSPTest {
         p = Parser.parse(" child-src 'self'", "http://example.com");
         assertTrue(p.allowsChildFromSource(URI.parse("http://example.com")));
         assertTrue(p.allowsFrameFromSource(URI.parse("http://example.com")));
-        assertFalse(p.allowsWorkerFromSource(URI.parse("http://example.com")));
-        assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com")));
+        assertTrue(p.allowsWorkerFromSource(URI.parse("http://example.com")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com")));
 
         p = Parser.parse(" child-src blob:", "http://example.com");
         assertTrue(p.allowsChildFromSource(new GUID("blob:")));
@@ -1304,8 +1304,8 @@ public class PolicyQueryingTest extends CSPTest {
         assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com")));
 
         p = Parser.parse("script-src 'none'; worker-src 'self'", "http://example.com");
-        assertFalse(p.allowsChildFromSource(URI.parse("http://example.com")));
-        assertFalse(p.allowsFrameFromSource(URI.parse("http://example.com")));
+        assertTrue(p.allowsChildFromSource(URI.parse("http://example.com")));
+        assertTrue(p.allowsFrameFromSource(URI.parse("http://example.com")));
         assertTrue(p.allowsWorkerFromSource(URI.parse("http://example.com")));
         assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com")));
 
@@ -1316,8 +1316,8 @@ public class PolicyQueryingTest extends CSPTest {
         assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com")));
 
         p = Parser.parse(" script-src 'self'", "http://example.com");
-        assertFalse(p.allowsChildFromSource(URI.parse("http://example.com")));
-        assertFalse(p.allowsFrameFromSource(URI.parse("http://example.com")));
+        assertTrue(p.allowsChildFromSource(URI.parse("http://example.com")));
+        assertTrue(p.allowsFrameFromSource(URI.parse("http://example.com")));
         assertTrue(p.allowsWorkerFromSource(URI.parse("http://example.com")));
         assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com")));