Skip to content

Commit

Permalink
Revert "fixes #202 - wrong validation of HashSource (#203)" (#204)
Browse files Browse the repository at this point in the history 
This reverts commit 527e55e.
  • Loading branch information
@shekyan @michaelficarra
shekyan authored and michaelficarra committed Aug 2, 2018
1 parent 527e55e commit 3813235
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 50 deletions.
6 changes: 3 additions & 3 deletions src/main/java/com/shapesecurity/salvation/directiveValues/HashSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,17 @@ public HashSource(@Nonnull HashAlgorithm algorithm, @Nonnull Base64Value value)
public void validationErrors() {
switch (this.algorithm) {
case SHA256:
if (this.value.size() != 64) {
if (this.value.size() != 32) {
throw new IllegalArgumentException("Invalid SHA-256 value (wrong length): " + this.value.size() + ".");
}
break;
case SHA384:
if (this.value.size() != 96) {
if (this.value.size() != 48) {
throw new IllegalArgumentException("Invalid SHA-384 value (wrong length): " + this.value.size() + ".");
}
break;
case SHA512:
if (this.value.size() != 128) {
if (this.value.size() != 64) {
throw new IllegalArgumentException("Invalid SHA-512 value (wrong length): " + this.value.size() + ".");
}
break;
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/com/shapesecurity/salvation/Base64ValueTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public void testIllegalDecodedSize() {
notices.get(0).show());

notices.clear();
Parser.parse("script-src 'self' 'sha256-YWFmMzU3YWU0ZDYzM2IzYWEzZTIzOTg2Yjk1ZGFjYWQ2Yzg_ZDdhZDM4MTAyZWUwMjNmZjk5M2IwNW-zN2RkOA==' https://example.com",
Parser.parse("script-src 'self' 'sha256-K7gNU3sdo-OL0wNhqoVWhr3g6s1xYv72ol_pe_Unols=' https://example.com",
"https://origin", notices);
assertEquals(1, notices.size());
assertEquals(
Expand Down
30 changes: 15 additions & 15 deletions src/test/java/com/shapesecurity/salvation/ParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,8 @@ public void testSourceExpressionParsing() {
assertEquals("optimisation with mixed schemes", "script-src custom: blob: *", parseAndShow("script-src 'self' * custom: ftp: blob:"));
assertEquals("optimisation", "script-src 'unsafe-inline' *", parseAndShow("script-src example.com * 'unsafe-inline'"));
assertEquals("optimisation", "script-src 'nonce-123' *", parseAndShow("script-src example.com * 'unsafe-inline' 'nonce-123'"));
assertEquals("optimisation", "script-src 'sha256-OWY4NmQwODE4ODRjN2Q2NTlhMmZlYWEwYzU1YWQwMTVhM2JmNGYxYjJiMGI4MjJjZDE1ZDZjMTViMGYwMGEwOA==' *", parseAndShow("script-src example.com * 'unsafe-inline' 'sha256-OWY4NmQwODE4ODRjN2Q2NTlhMmZlYWEwYzU1YWQwMTVhM2JmNGYxYjJiMGI4MjJjZDE1ZDZjMTViMGYwMGEwOA=='"));
assertEquals("optimisation", "script-src 'sha256-OWY4NmQwODE4ODRjN2Q2NTlhMmZlYWEwYzU1YWQwMTVhM2JmNGYxYjJiMGI4MjJjZDE1ZDZjMTViMGYwMGEwOA==' 'nonce-123' *", parseAndShow("script-src example.com * 'sha256-OWY4NmQwODE4ODRjN2Q2NTlhMmZlYWEwYzU1YWQwMTVhM2JmNGYxYjJiMGI4MjJjZDE1ZDZjMTViMGYwMGEwOA==' 'nonce-123' 'unsafe-inline'"));
assertEquals("optimisation", "script-src 'sha256-K7gNU3sdo+OL0wNhqoVWhr3g6s1xYv72ol/pe/Unols=' *", parseAndShow("script-src example.com * 'unsafe-inline' 'sha256-K7gNU3sdo+OL0wNhqoVWhr3g6s1xYv72ol/pe/Unols='"));
assertEquals("optimisation", "script-src 'sha256-K7gNU3sdo+OL0wNhqoVWhr3g6s1xYv72ol/pe/Unols=' 'nonce-123' *", parseAndShow("script-src example.com * 'sha256-K7gNU3sdo+OL0wNhqoVWhr3g6s1xYv72ol/pe/Unols=' 'nonce-123' 'unsafe-inline'"));

p = parse("script-src a; style-src a; img-src a; child-src a; connect-src a; font-src a; media-src a; object-src a; manifest-src a ");
assertEquals("script-src a; style-src a; img-src a; child-src a; connect-src a; font-src a; media-src a; object-src a; manifest-src a", p.show());
Expand Down Expand Up @@ -564,23 +564,23 @@ public void testHashSource() {
notices.get(0).message);

assertEquals("directive-name, directive-value",
"script-src 'self' https://example.com 'sha256-YWFmMzU3YWU0ZDYzM2IzYWEzZTIzOTg2Yjk1ZGFjYWQ2YzgyZDdhZDM4MTAyZWUwMjNmZjk5M2IwNWUzN2RkOA=='",
parse("script-src 'self' https://example.com 'sha256-YWFmMzU3YWU0ZDYzM2IzYWEzZTIzOTg2Yjk1ZGFjYWQ2YzgyZDdhZDM4MTAyZWUwMjNmZjk5M2IwNWUzN2RkOA=='")
"script-src 'self' https://example.com 'sha256-K7gNU3sdo+OL0wNhqoVWhr3g6s1xYv72ol/pe/Unols='",
parse("script-src 'self' https://example.com 'sha256-K7gNU3sdo+OL0wNhqoVWhr3g6s1xYv72ol/pe/Unols='")
.getDirectiveByType(ScriptSrcDirective.class).show());
assertEquals("directive-name, directive-value",
"script-src 'self' https://example.com 'sha384-NzY4NDEyMzIwZjdiMGFhNTgxMmZjZTQyOGRjNDcwNmIzY2FlNTBlMDJhNjRjYWExNmE3ODIyNDliZmU4ZWZjNGI3ZWYxY2NiMTI2MjU1ZDE5NjA0N2RmZWRmMTdhMGE5'",
"script-src 'self' https://example.com 'sha384-QXIS/RyLxYlv79jbWK+CRUXoWw0FRkCTZqMK73Jp+uJYFzvRhfsmLIbzu4b7oENo'",
parse(
"script-src 'self' https://example.com 'sha384-NzY4NDEyMzIwZjdiMGFhNTgxMmZjZTQyOGRjNDcwNmIzY2FlNTBlMDJhNjRjYWExNmE3ODIyNDliZmU4ZWZjNGI3ZWYxY2NiMTI2MjU1ZDE5NjA0N2RmZWRmMTdhMGE5'")
"script-src 'self' https://example.com 'sha384-QXIS/RyLxYlv79jbWK+CRUXoWw0FRkCTZqMK73Jp+uJYFzvRhfsmLIbzu4b7oENo'")
.getDirectiveByType(ScriptSrcDirective.class).show());
assertEquals("directive-name, directive-value",
"script-src 'self' https://example.com 'sha512-ZWUyNmIwZGQ0YWY3ZTc0OWFhMWE4ZWUzYzEwYWU5OTIzZjYxODk4MDc3MmU0NzNmODgxOWE1ZDQ5NDBlMGRiMjdhYzE4NWY4YTBlMWQ1Zjg0Zjg4YmM4ODdmZDY3YjE0MzczMmMzMDRjYzVmYTlhZDhlNmY1N2Y1MDAyOGE4ZmY='",
"script-src 'self' https://example.com 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='",
parse(
"script-src 'self' https://example.com 'sha512-ZWUyNmIwZGQ0YWY3ZTc0OWFhMWE4ZWUzYzEwYWU5OTIzZjYxODk4MDc3MmU0NzNmODgxOWE1ZDQ5NDBlMGRiMjdhYzE4NWY4YTBlMWQ1Zjg0Zjg4YmM4ODdmZDY3YjE0MzczMmMzMDRjYzVmYTlhZDhlNmY1N2Y1MDAyOGE4ZmY='")
"script-src 'self' https://example.com 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='")
.getDirectiveByType(ScriptSrcDirective.class).show());
p = parse(
"script-src 'sha512-ZWUyNmIwZGQ0YWY3ZTc0OWFhMWE4ZWUzYzEwYWU5OTIzZjYxODk4MDc3MmU0NzNmODgxOWE1ZDQ5NDBlMGRiMjdhYzE4NWY4YTBlMWQ1Zjg0Zjg4YmM4ODdmZDY3YjE0MzczMmMzMDRjYzVmYTlhZDhlNmY1N2Y1MDAyOGE4ZmY='");
"script-src 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='");
Policy q = parse(
"script-src 'sha512-ZWUyNmIwZGQ0YWY3ZTc0OWFhMWE4ZWUzYzEwYWU5OTIzZjYxODk4MDc3MmU0NzNmODgxOWE1ZDQ5NDBlMGRiMjdhYzE4NWY4YTBlMWQ1Zjg0Zjg4YmM4ODdmZDY3YjE0MzczMmMzMDRjYzVmYTlhZDhlNmY1N2Y1MDAyOGE4ZmY='");
"script-src 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='");
assertEquals("hash-source hashcode equality", p.hashCode(), q.hashCode());
ScriptSrcDirective d = p.getDirectiveByType(ScriptSrcDirective.class);
assertTrue("hash-source equals", d.equals(q.getDirectiveByType(ScriptSrcDirective.class)));
Expand Down Expand Up @@ -1051,7 +1051,7 @@ public void testUnsafeInlineWithHashNonce() {
assertEquals("The \"'unsafe-inline'\" keyword-source has no effect in source lists that contain hash-source or nonce-source in CSP2 and later. Ensure that this pattern is only used for backwards compatibility with older CSP implementations and is not an oversight.", notices.get(1).message);

notices.clear();
p = parseWithNotices("default-src 'unsafe-inline' 'sha512-ZWUyNmIwZGQ0YWY3ZTc0OWFhMWE4ZWUzYzEwYWU5OTIzZjYxODk4MDc3MmU0NzNmODgxOWE1ZDQ5NDBlMGRiMjdhYzE4NWY4YTBlMWQ1Zjg0Zjg4YmM4ODdmZDY3YjE0MzczMmMzMDRjYzVmYTlhZDhlNmY1N2Y1MDAyOGE4ZmY='", notices);
p = parseWithNotices("default-src 'unsafe-inline' 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='", notices);
assertEquals(1, p.getDirectives().size());
assertEquals(1, notices.size());
assertEquals("The \"'unsafe-inline'\" keyword-source has no effect in source lists that contain hash-source or nonce-source in CSP2 and later. Ensure that this pattern is only used for backwards compatibility with older CSP implementations and is not an oversight.", notices.get(0).message);
Expand All @@ -1072,7 +1072,7 @@ public void testUnsafeInlineWithHashNonce() {
public void testUnsafeHashedAttributes() {
Policy p;
ArrayList<Notice> notices = new ArrayList<>();
p = parseWithNotices("default-src 'unsafe-hashed-attributes' 'sha512-ZWUyNmIwZGQ0YWY3ZTc0OWFhMWE4ZWUzYzEwYWU5OTIzZjYxODk4MDc3MmU0NzNmODgxOWE1ZDQ5NDBlMGRiMjdhYzE4NWY4YTBlMWQ1Zjg0Zjg4YmM4ODdmZDY3YjE0MzczMmMzMDRjYzVmYTlhZDhlNmY1N2Y1MDAyOGE4ZmY='", notices);
p = parseWithNotices("default-src 'unsafe-hashed-attributes' 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='", notices);
assertEquals(1, p.getDirectives().size());
assertEquals(0, notices.size());

Expand Down Expand Up @@ -1100,20 +1100,20 @@ public void testUnsafeHashedAttributes() {
assertEquals("The \"'unsafe-hashed-attributes'\" keyword-source has no effect in source lists that do not contain hash-source in CSP3 and later.", notices.get(3).message);

notices.clear();
p = parseWithNotices("default-src 'unsafe-hashed-attributes' 'unsafe-hashed-attributes' 'unsafe-hashed-attributes' 'sha512-ZWUyNmIwZGQ0YWY3ZTc0OWFhMWE4ZWUzYzEwYWU5OTIzZjYxODk4MDc3MmU0NzNmODgxOWE1ZDQ5NDBlMGRiMjdhYzE4NWY4YTBlMWQ1Zjg0Zjg4YmM4ODdmZDY3YjE0MzczMmMzMDRjYzVmYTlhZDhlNmY1N2Y1MDAyOGE4ZmY='", notices);
p = parseWithNotices("default-src 'unsafe-hashed-attributes' 'unsafe-hashed-attributes' 'unsafe-hashed-attributes' 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='", notices);
assertEquals(1, p.getDirectives().size());
assertEquals(2, notices.size());
assertEquals("Source list contains duplicate source expression \"'unsafe-hashed-attributes'\". All but the first instance will be ignored.", notices.get(0).message);
assertEquals("Source list contains duplicate source expression \"'unsafe-hashed-attributes'\". All but the first instance will be ignored.", notices.get(1).message);

notices.clear();
p = parseWithNotices("default-src 'sha512-ZWUyNmIwZGQ0YWY3ZTc0OWFhMWE4ZWUzYzEwYWU5OTIzZjYxODk4MDc3MmU0NzNmODgxOWE1ZDQ5NDBlMGRiMjdhYzE4NWY4YTBlMWQ1Zjg0Zjg4YmM4ODdmZDY3YjE0MzczMmMzMDRjYzVmYTlhZDhlNmY1N2Y1MDAyOGE4ZmY='", notices);
p = parseWithNotices("default-src 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='", notices);
assertEquals(1, p.getDirectives().size());
assertEquals(0, notices.size());

// while grammar allows this, I am open to throw warnings about directives that don't make sense with 'usnafe-hashed-attributes'
notices.clear();
p = parseWithNotices("img-src 'unsafe-hashed-attributes' 'sha512-ZWUyNmIwZGQ0YWY3ZTc0OWFhMWE4ZWUzYzEwYWU5OTIzZjYxODk4MDc3MmU0NzNmODgxOWE1ZDQ5NDBlMGRiMjdhYzE4NWY4YTBlMWQ1Zjg0Zjg4YmM4ODdmZDY3YjE0MzczMmMzMDRjYzVmYTlhZDhlNmY1N2Y1MDAyOGE4ZmY='", notices);
p = parseWithNotices("img-src 'unsafe-hashed-attributes' 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='", notices);
assertEquals(1, p.getDirectives().size());
assertEquals(0, notices.size());
}
Expand Down
Loading

0 comments on commit 3813235

Please sign in to comment.