Skip to content

Commit

Permalink
fixes GH-129: keep non-network schemes during source-list optimisation
Browse files Browse the repository at this point in the history
  • Loading branch information
@shekyan
shekyan committed Apr 23, 2016
1 parent d2456a5 commit 60111fd
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 3 deletions.
4 changes: 2 additions & 2 deletions src/main/java/com/shapesecurity/salvation/data/Policy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@ private void optimise() {
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));
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/com/shapesecurity/salvation/directiveValues/SchemeSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
5 changes: 4 additions & 1 deletion src/test/java/com/shapesecurity/salvation/ParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:"));

}

Expand Down
44 changes: 44 additions & 0 deletions src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:")));
Expand All @@ -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:")));


}

Expand All @@ -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")));
Expand All @@ -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")));
Expand All @@ -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")));
Expand All @@ -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")));
Expand All @@ -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")));
Expand All @@ -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")));
Expand All @@ -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")));
Expand All @@ -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")));
Expand All @@ -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")));
Expand All @@ -415,15 +453,21 @@ 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")));
assertTrue(p.allowsScriptFromSource(URI.parse("http://a.b.example.com")));
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:")));
}

}

0 comments on commit 60111fd

Please sign in to comment.