Skip to content

Commit

Permalink
fixes #91: allow secure variant of scheme when only insecure is given
Browse files Browse the repository at this point in the history
  • Loading branch information
@shekyan
shekyan committed Dec 1, 2016
1 parent 7fe84bd commit 10ee24d
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 43 deletions.
18 changes: 18 additions & 0 deletions src/main/java/com/shapesecurity/salvation/data/SchemeHostPortTriple.java
View file
Expand Up @@ -60,4 +60,22 @@ public static int defaultPortForProtocol(@Nonnull String scheme) {
this.host +
(isDefaultPort ? "" : ":" + this.port);
}

public static boolean isSchemeNetworkScheme(String scheme) {
return scheme != null && (scheme.equalsIgnoreCase("ftp") || scheme.equalsIgnoreCase("http") ||
scheme.equalsIgnoreCase("https") || scheme.equalsIgnoreCase("ws") ||
scheme.equalsIgnoreCase("wss"));
}

public static boolean isSchemeSecureScheme(String scheme) {
return scheme != null && (scheme.equalsIgnoreCase("https") || scheme.equalsIgnoreCase("wss"));
}

public boolean isNetworkScheme() {
return isSchemeNetworkScheme(this.scheme);
}

public boolean isSecureScheme() {
return isSchemeSecureScheme(this.scheme);
}
}
6 changes: 0 additions & 6 deletions src/main/java/com/shapesecurity/salvation/data/URI.java
View file
Expand Up @@ -70,12 +70,6 @@ public URI(@Nonnull SchemeHostPortTriple origin) {
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;
}
Expand Down
16 changes: 12 additions & 4 deletions src/main/java/com/shapesecurity/salvation/directiveValues/HostSource.java
View file
Expand Up @@ -72,11 +72,18 @@ public boolean isWildcard() {
}
boolean schemeMatches;
if (this.scheme == null) {
schemeMatches = resource.scheme.equalsIgnoreCase("http") ?
shpOrigin.scheme.equalsIgnoreCase("http") || shpOrigin.scheme.equalsIgnoreCase("https") :
schemeMatches = resource.isNetworkScheme() ?
shpOrigin.isNetworkScheme() :
resource.scheme.equalsIgnoreCase(shpOrigin.scheme);
} else {
schemeMatches = this.scheme.equalsIgnoreCase(resource.scheme);
if (resource.isNetworkScheme() && !resource.scheme.equalsIgnoreCase("ftp") && !this.scheme.equalsIgnoreCase("ftp")) {
schemeMatches = resource.isSecureScheme() ?
SchemeHostPortTriple.isSchemeNetworkScheme(this.scheme) :
!SchemeHostPortTriple.isSchemeSecureScheme(this.scheme);

} else {
schemeMatches = this.scheme.equalsIgnoreCase(resource.scheme);
}
}
boolean hostMatches = this.host.equals("*") || (this.host.startsWith("*.") ?
resource.host.toLowerCase().endsWith(this.host.substring(1).toLowerCase()) :
Expand All @@ -85,7 +92,8 @@ public boolean isWildcard() {
|| 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 ?
boolean portMatches = this.port == Constants.WILDCARD_PORT || (thisUsesDefaultPort && uriUsesDefaultPort) ||
(this.port == Constants.EMPTY_PORT ?
uriUsesDefaultPort :
(resource.port == Constants.EMPTY_PORT ? thisUsesDefaultPort : this.port == resource.port));
boolean pathMatches = this.path == null || (this.path.endsWith("/") ?
Expand Down
102 changes: 69 additions & 33 deletions src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
View file
Expand Up @@ -103,8 +103,8 @@ public class PolicyQueryingTest extends CSPTest {

p = Parser.parse("default-src *:*", "http://abc.com");
assertTrue("resource is allowed", p.allowsImgFromSource(URI.parse("http://abc.am")));
assertFalse("resource is not allowed", p.allowsScriptFromSource(URI.parse("https://www.def.am:555")));
assertFalse("resource is not allowed", p.allowsStyleFromSource(URI.parse("ftp://www.abc.am:555")));
assertTrue("resource is allowed", p.allowsScriptFromSource(URI.parse("https://www.def.am:555")));
assertTrue("resource is allowed", p.allowsStyleFromSource(URI.parse("ftp://www.abc.am:555")));

p = Parser.parse("default-src 'none'; frame-src http:;", URI.parse("https://abc.com"));
assertFalse("resource is not allowed", p.allowsFrameFromSource(URI.parse("https://www.def.am:555")));
Expand Down Expand Up @@ -149,6 +149,42 @@ public class PolicyQueryingTest extends CSPTest {

}

@Test public void testSecureSchemes() {
Policy p;
p = Parser.parse("script-src a;", "http://example.com");
assertTrue(p.allowsScriptFromSource(URI.parse("https://a")));

p = Parser.parse("script-src https://a;", "http://example.com");
assertFalse(p.allowsScriptFromSource(URI.parse("http://a")));

p = Parser.parse("script-src http://a;", "https://example.com");
assertTrue(p.allowsScriptFromSource(URI.parse("http://a")));

p = Parser.parse("script-src http://a;", "http://example.com");
assertTrue(p.allowsScriptFromSource(URI.parse("http://a")));

p = Parser.parse("script-src http://a;", "http://example.com");
assertTrue(p.allowsScriptFromSource(URI.parse("https://a")));

p = Parser.parse("script-src https://a;", "http://example.com");
assertTrue(p.allowsScriptFromSource(URI.parse("https://a")));

p = Parser.parse("script-src ws://a;", "http://example.com");
assertTrue(p.allowsScriptFromSource(URI.parse("https://a")));

p = Parser.parse("script-src wss://a;", "http://example.com");
assertTrue(p.allowsScriptFromSource(URI.parse("https://a")));

p = Parser.parse("script-src wss://a;", "http://example.com");
assertFalse(p.allowsScriptFromSource(URI.parse("ws://a")));

p = Parser.parse("script-src wss://a;", "http://example.com");
assertFalse(p.allowsScriptFromSource(URI.parse("http://a")));

p = Parser.parse("script-src ws://a;", "http://example.com");
assertTrue(p.allowsScriptFromSource(URI.parse("https://a")));
}

@Test public void testAllowsUnsafeInline() {
Policy p;

Expand Down Expand Up @@ -250,7 +286,7 @@ public class PolicyQueryingTest extends CSPTest {
p = Parser.parse("default-src *:* 'unsafe-inline'; connect-src 'self' http://good.com/", "https://abc.com");
assertTrue("connect is allowed", p.allowsConnectTo(URI.parse("https://abc.com")));
assertTrue("connect is allowed", p.allowsConnectTo(URI.parse("http://good.com/")));
assertFalse("connect is not allowed", p.allowsConnectTo(URI.parse("https://good.com/")));
assertTrue("connect is allowed", p.allowsConnectTo(URI.parse("https://good.com/")));
assertFalse("connect is not allowed", p.allowsConnectTo(URI.parse("http://aaa.good.com/")));
assertFalse("connect is not allowed", p.allowsConnectTo(URI.parse("wss://abc.com/")));
assertFalse("connect is not allowed", p.allowsConnectTo(URI.parse("http://abc.com/")));
Expand Down Expand Up @@ -445,14 +481,14 @@ public class PolicyQueryingTest extends CSPTest {

p = Parser.parse("script-src http://*", "http://example.com");
assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com")));
assertFalse(p.allowsScriptFromSource(URI.parse("https://example.com")));
assertTrue(p.allowsScriptFromSource(URI.parse("https://example.com")));
assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com:81")));
assertFalse(p.allowsScriptFromSource(URI.parse("ftp://example.com")));
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")));
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:")));

Expand All @@ -461,10 +497,10 @@ public class PolicyQueryingTest extends CSPTest {
assertFalse(p.allowsStyleFromSource(URI.parse("https://example.com")));
assertFalse(p.allowsStyleFromSource(URI.parse("http://example.com:81")));
assertFalse(p.allowsStyleFromSource(URI.parse("ftp://example.com")));
assertFalse(p.allowsStyleFromSource(URI.parse("ftp://example.com:80")));
assertTrue(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")));
assertTrue(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:")));
Expand All @@ -474,21 +510,21 @@ public class PolicyQueryingTest extends CSPTest {
assertFalse(p.allowsStyleFromSource(URI.parse("https://example.com")));
assertFalse(p.allowsStyleFromSource(URI.parse("http://example.com:81")));
assertFalse(p.allowsStyleFromSource(URI.parse("ftp://example.com")));
assertFalse(p.allowsStyleFromSource(URI.parse("ftp://example.com:80")));
assertTrue(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")));
assertTrue(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")));
assertTrue(p.allowsStyleFromSource(URI.parse("http://example.com")));
assertFalse(p.allowsStyleFromSource(URI.parse("https://example.com")));
assertFalse(p.allowsStyleFromSource(URI.parse("http://example.com:81")));
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")));
assertTrue(p.allowsStyleFromSource(URI.parse("http://example.com/path")));
assertTrue(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:")));
Expand All @@ -507,25 +543,25 @@ public class PolicyQueryingTest extends CSPTest {

p = Parser.parse("style-src *:*", "http://example.com");
assertTrue(p.allowsStyleFromSource(URI.parse("http://example.com")));
assertFalse(p.allowsStyleFromSource(URI.parse("https://example.com")));
assertTrue(p.allowsStyleFromSource(URI.parse("https://example.com")));
assertTrue(p.allowsStyleFromSource(URI.parse("http://example.com:81")));
assertFalse(p.allowsStyleFromSource(URI.parse("ftp://example.com")));
assertFalse(p.allowsStyleFromSource(URI.parse("ftp://example.com:80")));
assertTrue(p.allowsStyleFromSource(URI.parse("ftp://example.com")));
assertTrue(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")));
assertTrue(p.allowsStyleFromSource(URI.parse("ws://example.com/PATH")));
assertTrue(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")));
assertFalse(p.allowsStyleFromSource(URI.parse("https://example.com")));
assertTrue(p.allowsStyleFromSource(URI.parse("https://example.com")));
assertTrue(p.allowsStyleFromSource(URI.parse("http://example.com:81")));
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")));
assertTrue(p.allowsStyleFromSource(URI.parse("ws://example.com/PATH")));
assertTrue(p.allowsStyleFromSource(URI.parse("wss://example.com/PATH")));
assertFalse(p.allowsStyleFromSource(new GUID("data:")));
assertFalse(p.allowsStyleFromSource(new GUID("custom.scheme:")));

Expand Down Expand Up @@ -579,14 +615,14 @@ public class PolicyQueryingTest extends CSPTest {

p = Parser.parse("font-src http://*", "http://example.com");
assertTrue(p.allowsFontFromSource(URI.parse("http://example.com")));
assertFalse(p.allowsFontFromSource(URI.parse("https://example.com")));
assertTrue(p.allowsFontFromSource(URI.parse("https://example.com")));
assertFalse(p.allowsFontFromSource(URI.parse("http://example.com:81")));
assertFalse(p.allowsFontFromSource(URI.parse("ftp://example.com")));
assertFalse(p.allowsFontFromSource(URI.parse("ftp://example.com:80")));
assertTrue(p.allowsFontFromSource(URI.parse("http://example.com/path")));
assertTrue(p.allowsFontFromSource(URI.parse("http://example.com/PATH")));
assertFalse(p.allowsFontFromSource(URI.parse("ws://example.com/PATH")));
assertFalse(p.allowsFontFromSource(URI.parse("wss://example.com/PATH")));
assertTrue(p.allowsFontFromSource(URI.parse("ws://example.com/PATH")));
assertTrue(p.allowsFontFromSource(URI.parse("wss://example.com/PATH")));
assertFalse(p.allowsFontFromSource(new GUID("data:")));
assertFalse(p.allowsFontFromSource(new GUID("custom.scheme:")));

Expand Down Expand Up @@ -616,14 +652,14 @@ public class PolicyQueryingTest extends CSPTest {

p = Parser.parse("object-src http://*", "http://example.com");
assertTrue(p.allowsObjectFromSource(URI.parse("http://example.com")));
assertFalse(p.allowsObjectFromSource(URI.parse("https://example.com")));
assertTrue(p.allowsObjectFromSource(URI.parse("https://example.com")));
assertFalse(p.allowsObjectFromSource(URI.parse("http://example.com:81")));
assertFalse(p.allowsObjectFromSource(URI.parse("ftp://example.com")));
assertFalse(p.allowsObjectFromSource(URI.parse("ftp://example.com:80")));
assertTrue(p.allowsObjectFromSource(URI.parse("http://example.com/path")));
assertTrue(p.allowsObjectFromSource(URI.parse("http://example.com/PATH")));
assertFalse(p.allowsObjectFromSource(URI.parse("ws://example.com/PATH")));
assertFalse(p.allowsObjectFromSource(URI.parse("wss://example.com/PATH")));
assertTrue(p.allowsObjectFromSource(URI.parse("ws://example.com/PATH")));
assertTrue(p.allowsObjectFromSource(URI.parse("wss://example.com/PATH")));
assertFalse(p.allowsObjectFromSource(new GUID("data:")));
assertFalse(p.allowsObjectFromSource(new GUID("custom.scheme:")));

Expand Down Expand Up @@ -653,14 +689,14 @@ public class PolicyQueryingTest extends CSPTest {

p = Parser.parse("media-src http://*", "http://example.com");
assertTrue(p.allowsMediaFromSource(URI.parse("http://example.com")));
assertFalse(p.allowsMediaFromSource(URI.parse("https://example.com")));
assertTrue(p.allowsMediaFromSource(URI.parse("https://example.com")));
assertFalse(p.allowsMediaFromSource(URI.parse("http://example.com:81")));
assertFalse(p.allowsMediaFromSource(URI.parse("ftp://example.com")));
assertFalse(p.allowsMediaFromSource(URI.parse("ftp://example.com:80")));
assertTrue(p.allowsMediaFromSource(URI.parse("http://example.com/path")));
assertTrue(p.allowsMediaFromSource(URI.parse("http://example.com/PATH")));
assertFalse(p.allowsMediaFromSource(URI.parse("ws://example.com/PATH")));
assertFalse(p.allowsMediaFromSource(URI.parse("wss://example.com/PATH")));
assertTrue(p.allowsMediaFromSource(URI.parse("ws://example.com/PATH")));
assertTrue(p.allowsMediaFromSource(URI.parse("wss://example.com/PATH")));
assertFalse(p.allowsMediaFromSource(new GUID("data:")));
assertFalse(p.allowsMediaFromSource(new GUID("custom.scheme:")));

Expand Down Expand Up @@ -690,14 +726,14 @@ public class PolicyQueryingTest extends CSPTest {

p = Parser.parse("manifest-src http://*", "http://example.com");
assertTrue(p.allowsManifestFromSource(URI.parse("http://example.com")));
assertFalse(p.allowsManifestFromSource(URI.parse("https://example.com")));
assertTrue(p.allowsManifestFromSource(URI.parse("https://example.com")));
assertFalse(p.allowsManifestFromSource(URI.parse("http://example.com:81")));
assertFalse(p.allowsManifestFromSource(URI.parse("ftp://example.com")));
assertFalse(p.allowsManifestFromSource(URI.parse("ftp://example.com:80")));
assertTrue(p.allowsManifestFromSource(URI.parse("http://example.com/path")));
assertTrue(p.allowsManifestFromSource(URI.parse("http://example.com/PATH")));
assertFalse(p.allowsManifestFromSource(URI.parse("ws://example.com/PATH")));
assertFalse(p.allowsManifestFromSource(URI.parse("wss://example.com/PATH")));
assertTrue(p.allowsManifestFromSource(URI.parse("ws://example.com/PATH")));
assertTrue(p.allowsManifestFromSource(URI.parse("wss://example.com/PATH")));
assertFalse(p.allowsManifestFromSource(new GUID("data:")));
assertFalse(p.allowsManifestFromSource(new GUID("custom.scheme:")));

Expand Down

0 comments on commit 10ee24d

Please sign in to comment.