Closed Bug 1490165 Opened 5 years ago Closed 5 years ago

Web Workers setTimeout / setInterval CSP bypass

Categories

(Core :: DOM: Security, defect, P3)

62 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: francois.lajeunesse.robert, Assigned: baku)

References

Details

(Whiteboard: [domsecurity-backlog1])

Attachments

(3 files, 1 obsolete file)

Attached image Worker_setTimeout_CSP_bypass.png (obsolete) —
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180830143136

Steps to reproduce:

When a Web worker javascript file (either a Worker, SharedWorker or ServiceWorker) is loaded with its own Content-Security-Policy set to default-src 'self', the CSP could be bypassed by either calling the setTimeout or the setInterval function.


Actual results:

Loading a Web worker from a server reponse with a Content-Security-Policy set to default-src 'self' and the following instruction (see the screenshot in Worker_setTimeout_CSP_bypass.png)
  setTimeout("console.log('[FAIL] - CSP bypass setTimeout')")
will display an entry in the console log which indicates that an "eval" as occurred even if the 'unsafe-eval' instruction is not set in the Content-Security-Policy.


Expected results:

The CSP should have blocked the execution of the setTimeout instruction as "eval" or "new Function()" instructions.
Note that the setTimeout / setInterval CSP bypass also works for a Blob URL with an inherited Content-Security-Policy default-src 'self' blob:
Group: firefox-core-security → dom-core-security
Component: Untriaged → DOM: Security
Product: Firefox → Core
Blocks: 1490171
These are known failures in the Web Platform Tests for this feature:
https://searchfox.org/mozilla-central/source/testing/web-platform/meta/content-security-policy/inside-worker/dedicated-script.html.ini
No longer blocks: 1490171
Group: dom-core-security
un-hid bug because the Web Platform Test results are public:
https://wpt.fyi/results/content-security-policy/inside-worker?complete=true
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Whiteboard: [domsecurity-backlog1]
Assignee: nobody → amarchesini
Attachment #9007951 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #9008777 - Flags: review?(ckerschb)
Attached patch part 2 - evalSplinter Review
Attachment #9008778 - Flags: review?(ckerschb)
These 2 patches do not cover importScripts() yet.
Attachment #9008831 - Flags: review?(ckerschb)
Comment on attachment 9008777 [details] [diff] [review]
part 1 - SetTimeout/SetInternval in workers

Review of attachment 9008777 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks Baku, that looks great!

::: dom/security/CSPEvalChecker.cpp
@@ +27,5 @@
> +              bool* aAllowed)
> +{
> +  MOZ_ASSERT(NS_IsMainThread());
> +  MOZ_ASSERT(aAllowed);
> +

Can you please do
*aAllowed = false;
at the function entry point here to make sure in case someone extends the code the return val is always set - thanks.

@@ +102,5 @@
> +{
> +  MOZ_ASSERT(NS_IsMainThread());
> +  MOZ_ASSERT(aWindow);
> +  MOZ_ASSERT(aAllowEval);
> +

same here, please init
aAllowEval = false;
Attachment #9008777 - Flags: review?(ckerschb) → review+
Attachment #9008778 - Flags: review?(ckerschb) → review+
Attachment #9008831 - Flags: review?(ckerschb) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ebcf49549e65
Workers.setTimeout/setInterval must handle CSP rejections, r=ckerschb
https://hg.mozilla.org/integration/mozilla-inbound/rev/28025c893d78
CSP blocking function() must return an EvalError, r=ckerschb
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc8df9e94e81
WorkerPrivate must set the CSPEventListener at any CSP internal object, r=ckerschb
https://hg.mozilla.org/integration/mozilla-inbound/rev/f628cd83dd09
Mark a WPT as error expected - the test is wrong and it will be fixed in a follow up, r=me
Depends on: 1492392
Blocks: 1493629
Please request uplift here.
Flags: needinfo?(amarchesini)
Flags: needinfo?(amarchesini)
Might need a mention in the docs somewhere.
Keywords: dev-doc-needed
I'm not so convinced any more; more of a bug fix than a feature.
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.