From 60111fddbafb8a5ae16e8bc7fdc0fb787b1c1319 Mon Sep 17 00:00:00 2001 From: Sergey Shekyan Date: Tue, 19 Apr 2016 12:02:14 -0700 Subject: [PATCH] fixes GH-129: keep non-network schemes during source-list optimisation --- .../shapesecurity/salvation/data/Policy.java | 4 +- .../directiveValues/SchemeSource.java | 6 +++ .../shapesecurity/salvation/ParserTest.java | 5 ++- .../salvation/PolicyQueryingTest.java | 44 +++++++++++++++++++ 4 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/shapesecurity/salvation/data/Policy.java b/src/main/java/com/shapesecurity/salvation/data/Policy.java index 9dea5830..37d84735 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 @@ private void optimise() { Set 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 44b7b0ea..4d41c273 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 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") || + 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)) diff --git a/src/test/java/com/shapesecurity/salvation/ParserTest.java b/src/test/java/com/shapesecurity/salvation/ParserTest.java index b0f9c098..0c922a9b 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 a55b75aa..37cb1bb2 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:"))); } }