Bug 239840

Summary: REGRESSION (iOS 15.4): Worker csp with script-src 'strict-dynamic' and script-src-elem blocks importScripts that should pass
Product: WebKit Reporter: Julian M <webkit>
Component: Page LoadingAssignee: Patrick Griffis <pgriffis>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, bfulgham, ews-watchlist, katherine_cheney, mkwst, pgriffis, seongil.wi, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 15   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Julian M 2022-04-27 23:22:26 PDT 
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
Comment 1 Radar WebKit Bug Importer 2022-04-29 08:31:58 PDT 
<rdar://problem/92525351>
Comment 2 Kate Cheney 2022-04-29 08:33:40 PDT 
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?
Comment 3 Radar WebKit Bug Importer 2022-04-29 08:33:47 PDT Comment hidden (obsolete)
Comment 4 Julian M 2022-04-29 16:03:14 PDT 
(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.
Comment 5 Patrick Griffis 2022-04-30 10:44:30 PDT 
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)
Comment 6 Kate Cheney 2022-05-04 08:25:53 PDT 
(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.
Comment 7 Patrick Griffis 2022-05-05 14:04:22 PDT 
Created attachment 458912 [details]
Patch
Comment 8 EWS 2022-05-06 18:39:21 PDT 
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].
Comment 9 Brent Fulgham 2022-06-23 12:03:43 PDT 
*** Bug 241337 has been marked as a duplicate of this bug. ***