Bug 239840 – REGRESSION (iOS 15.4): Worker csp with script-src 'strict-dynamic' and script-src-elem blocks importScripts that should pass

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Julian M]

Hi team,

I have encountered an issue in Safari 15.4 on iOS and macOS where a worker loaded with a CSP header calls importScripts to bring in a local script and fails unexpectedly. I was serving a worker script with the same CSP as used elsewhere in an application and encountered this issue.

I managed to boil it down to a CSP that fails but it's not obvious to me why, I think it's a bug but apologies if not.

A worker loaded with the following CSP fails to load a script via importScripts:
default-src 'self'; script-src 'strict-dynamic'; script-src-elem 'self';

Removing either "'strict-dynamic'" OR "script-src-elem 'self';" resolves the issue, e.g. either:
default-src 'self'; script-src 'strict-dynamic';
default-src 'self'; script-src-elem 'self';


I have a small test case that loads a worker that calls importScripts, none of these cases fail in Chrome/Firefox/Opera/Edge latest.

https://www.tests.massivedev.com/safari-worker-csp/?csp=1 - this fails in Safari 15.4
https://www.tests.massivedev.com/safari-worker-csp/?csp=2
https://www.tests.massivedev.com/safari-worker-csp/?csp=3
https://www.tests.massivedev.com/safari-worker-csp/?csp=4

[Radar WebKit Bug Importer]

<rdar://problem/92525351>

[Kate Cheney]

Hi! Thank you for filing this! I just want to confirm I understand - this behavior worked as expected on previous versions of iOS and now fails on 15.4?

[Radar WebKit Bug Importer]

<rdar://problem/92525435>

[Julian M]

(In reply to Kate Cheney from comment #2)
> Hi! Thank you for filing this! I just want to confirm I understand - this
> behavior worked as expected on previous versions of iOS and now fails on
> 15.4?

Yep! I'm guessing based on the introduction of 'strict-dynamic' support.

[Patrick Griffis]

Didn't write a test for it yet and not sure this is the correct directive for everything that calls this method but the most direct fix:

diff --git a/Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp b/Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp
index 7d73ac8bfb97..c7466c36f437 100644
--- a/Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp
+++ b/Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp
@@ -408,8 +408,7 @@ const ContentSecurityPolicyDirective* ContentSecurityPolicyDirectiveList::violat
 
 const ContentSecurityPolicyDirective* ContentSecurityPolicyDirectiveList::violatedDirectiveForScript(const URL& url, bool didReceiveRedirectResponse, const Vector<ResourceCryptographicDigest>& subResourceIntegrityDigests, const String& nonce) const
 {
-    auto* operativeDirective = this->operativeDirective(m_scriptSrc.get(), ContentSecurityPolicyDirectiveNames::scriptSrcElem);
+    auto* operativeDirective = this->operativeDirective(m_scriptSrcElem.get(), ContentSecurityPolicyDirectiveNames::scriptSrcElem);

     if (!operativeDirective
         || operativeDirective->containsAllHashes(subResourceIntegrityDigests)
         || checkNonce(operativeDirective, nonce)

[Kate Cheney]

(In reply to Patrick Griffis from comment #5)
> Didn't write a test for it yet and not sure this is the correct directive
> for everything that calls this method but the most direct fix:
> 
> diff --git a/Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp
> b/Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp
> index 7d73ac8bfb97..c7466c36f437 100644
> --- a/Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp
> +++ b/Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp
> @@ -408,8 +408,7 @@ const ContentSecurityPolicyDirective*
> ContentSecurityPolicyDirectiveList::violat
>  
>  const ContentSecurityPolicyDirective*
> ContentSecurityPolicyDirectiveList::violatedDirectiveForScript(const URL&
> url, bool didReceiveRedirectResponse, const
> Vector<ResourceCryptographicDigest>& subResourceIntegrityDigests, const
> String& nonce) const
>  {
> -    auto* operativeDirective = this->operativeDirective(m_scriptSrc.get(),
> ContentSecurityPolicyDirectiveNames::scriptSrcElem);
> +    auto* operativeDirective =
> this->operativeDirective(m_scriptSrcElem.get(),
> ContentSecurityPolicyDirectiveNames::scriptSrcElem);
> 
>      if (!operativeDirective
>          ||
> operativeDirective->containsAllHashes(subResourceIntegrityDigests)
>          || checkNonce(operativeDirective, nonce)

I think this will be fine because violatedDirectiveForScript is called for non-inline script sources which should all fall under script-src-elem. We will need to use operativeDirectiveScript to ensure we fall back to script-src if script-src-elem is not present.

[Patrick Griffis]

Created attachment 458912 [details]
Patch

[EWS]

Committed r293940 (250386@main): <https://commits.webkit.org/250386@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 458912 [details].

[Brent Fulgham]

*** Bug 241337 has been marked as a duplicate of this bug. ***