Repository: shapesecurity/salvation
Pull request #224 (2019-07-18 18:48:46)

Make worker-src fall back in the correct order
- `worker-src` falls back to `child-src` and then `default-src`.
- `child-src` does not fall back to `script-src`, just `default-src`.
- The logic for `expandDefaultSrc` previously expanded `default-src` before anything else.

That meant that if there were a `default-src` and a `script-src`, but no `child-src`, then `worker-src` would fall back to `default-src` (via the synthetic `child-src`) instead of to the actual `script-src`. That was wrong.


bakkot: Also we really need to do #219.

Patch:
diff --git a/src/main/java/com/shapesecurity/salvation/data/Policy.java b/src/main/java/com/shapesecurity/salvation/data/Policy.java
index dfe65c5..d53afa8 100644
--- a/src/main/java/com/shapesecurity/salvation/data/Policy.java
+++ b/src/main/java/com/shapesecurity/salvation/data/Policy.java
@@ -111,6 +111,16 @@ public class Policy implements Show {
 	}
 
 	private void expandDefaultSrc() {
+		// This is a special case because worker-src falls back to child-src before script-src,
+		// but child-src does _not_ fall back to script-src.
+		// Hence this logic needs to happen before expanding anything into child-src, to ensure that
+		// if script-src is present and child-src is not, worker-src comes from script-src rather than default-src.
+		if (this.directives.containsKey(ScriptSrcDirective.class) && !this.directives.containsKey(ChildSrcDirective.class) && !this.directives.containsKey(WorkerSrcDirective.class)) {
+			ScriptSrcDirective scriptSrcDirective = this.getDirectiveByType(ScriptSrcDirective.class);
+			Set<SourceExpression> sources = scriptSrcDirective.values().collect(Collectors.toCollection(LinkedHashSet::new));
+			this.directives.put(WorkerSrcDirective.class, new WorkerSrcDirective(sources));
+		}
+
 		DefaultSrcDirective defaultSrcDirective = this.getDirectiveByType(DefaultSrcDirective.class);
 		Set<SourceExpression> defaultSources;
 		if (defaultSrcDirective != null) {


Test patch:
diff --git a/src/test/java/com/shapesecurity/salvation/PolicyMergeTest.java b/src/test/java/com/shapesecurity/salvation/PolicyMergeTest.java
index 14075c5..d9de964 100644
--- a/src/test/java/com/shapesecurity/salvation/PolicyMergeTest.java
+++ b/src/test/java/com/shapesecurity/salvation/PolicyMergeTest.java
@@ -29,13 +29,22 @@ public class PolicyMergeTest extends CSPTest {
 		p1.union(p2);
 		assertEquals("default-src aaa https://origin2.com", p1.show());
 
-		p1 = Parser.parse("default-src d; connect-src a; script-src a; media-src a; ", "https://origin1.com");
+		p1 = Parser.parse("default-src d; connect-src a; script-src a; media-src a;", "https://origin1.com");
 		p2 = Parser
 				.parse("default-src; img-src b; style-src b; font-src b; child-src b; object-src b; manifest-src b; prefetch-src b;", "https://origin2.com");
 		p1.union(p2);
 		assertEquals(
-				"connect-src a; script-src a; media-src a; style-src d b; img-src d b; child-src d b; font-src d b; object-src d b; manifest-src d b; prefetch-src d b",
+				"connect-src a; script-src a; media-src a; worker-src a b; style-src d b; img-src d b; child-src d b; font-src d b; object-src d b; manifest-src d b; prefetch-src d b",
 				p1.show());
+
+		p1 = Parser.parse("default-src aaa; script-src bbb", "https://origin1.com");
+		Set<SourceExpression> set = new LinkedHashSet<>();
+		set.add(new HostSource(null, "ccc", -1, null));
+		ScriptSrcDirective scriptSrcDirective = new ScriptSrcDirective(set);
+		p1.unionDirective(scriptSrcDirective);
+		assertEquals(
+			"default-src aaa; script-src bbb ccc",
+			p1.show());
 	}
 
 	@Test