1674343 - (CVE-2020-26976) Service worker can control a non-secure context client

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect, P2)

Description

[Yaron Tausky [:ytausky]]

An HTTPS iframe embedded in an HTTP page is loaded with a service worker (if one is registered), even though it's not a secure context.

Comment 1

[Yaron Tausky [:ytausky]]

Attachment: Bug 1674343 - Check for secure context when deciding to intercept r=asuth

Comment 2

[Yaron Tausky [:ytausky]]

Attachment: Bug 1674343 - Test that service workers respect secure context restrictions r=asuth

Depends on D96179

Comment 3

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Landed:
https://hg.mozilla.org/integration/autoland/rev/28b29531f0c0e32ac4e5304218e3c7198fa89f05

Backed out for mochitest serviceworker related failures:

https://hg.mozilla.org/integration/autoland/rev/a4610b94c6d477f5c456fd1b70863bde773fe501

Failures:
devtools: https://treeherder.mozilla.org/logviewer?job_id=321325365&repo=autoland

[task 2020-11-10T16:17:31.139Z] 16:17:31 INFO - GECKO(4366) | Hit MOZ_CRASH(Workers Hanging - 1|A:1|S:0|Q:0-BC:0IsChromeWorker(false)) at /builds/worker/checkouts/gecko/dom/workers/RuntimeService.cpp:1697
[task 2020-11-10T16:17:31.143Z] 16:17:31 INFO - Initializing stack-fixing for the first stack frame, this may take a while...
[task 2020-11-10T16:17:36.884Z] 16:17:36 INFO - GECKO(4366) | #01: mozilla::(anonymous namespace)::RunWatchdog(void*) [toolkit/components/terminator/nsTerminator.cpp:219]
[task 2020-11-10T16:17:36.900Z] 16:17:36 INFO - GECKO(4366) | #02: _pt_root [nsprpub/pr/src/pthreads/ptthread.c:204]
[task 2020-11-10T16:17:36.901Z] 16:17:36 INFO - fix-stacks: error: failed to read symbols file /builds/worker/workspace/build/symbols/libpthread.so.0/10063CBC74776C265F7DB3F1F380A3DA0/libpthread.so.0.sym for /lib/x86_64-linux-gnu/libpthread.so.0
[task 2020-11-10T16:17:36.902Z] 16:17:36 INFO - fix-stacks: note: this is expected and harmless for system libraries on debug automation runs
[task 2020-11-10T16:17:36.903Z] 16:17:36 INFO - fix-stacks: No such file or directory (os error 2)
[task 2020-11-10T16:17:36.904Z] 16:17:36 INFO - GECKO(4366) | #03: ??? [/lib/x86_64-linux-gnu/libpthread.so.0 + 0x76db]
[task 2020-11-10T16:17:36.904Z] 16:17:36 INFO - fix-stacks: error: failed to read symbols file /builds/worker/workspace/build/symbols/libc.so.6/4B76CFD3972F3EACFE366DDD07AD902F0/libc.so.6.sym for /lib/x86_64-linux-gnu/libc.so.6
[task 2020-11-10T16:17:36.905Z] 16:17:36 INFO - fix-stacks: note: this is expected and harmless for system libraries on debug automation runs
[task 2020-11-10T16:17:36.905Z] 16:17:36 INFO - fix-stacks: No such file or directory (os error 2)
[task 2020-11-10T16:17:36.905Z] 16:17:36 INFO - GECKO(4366) | #04: clone [/lib/x86_64-linux-gnu/libc.so.6 + 0x121a3f]
[task 2020-11-10T16:17:36.905Z] 16:17:36 INFO - GECKO(4366) | #05: ??? (???:???)
[task 2020-11-10T16:17:36.905Z] 16:17:36 INFO - GECKO(4366) | ExceptionHandler::GenerateDump cloned child 4827
[task 2020-11-10T16:17:36.905Z] 16:17:36 INFO - GECKO(4366) | ExceptionHandler::SendContinueSignalToChild sent continue signal to child
[task 2020-11-10T16:17:36.905Z] 16:17:36 INFO - GECKO(4366) | ExceptionHandler::WaitForContinueSignal waiting for continue signal...

browser-chrome: https://treeherder.mozilla.org/logviewer?job_id=321325993&repo=autoland
mochitest plain: https://treeherder.mozilla.org/logviewer?job_id=321325948&repo=autoland

Comment 4

[Yaron Tausky [:ytausky]]

I forgot to enable interception for non-secure contexts if the dom.serviceWorkers.testing.enabled pref is enabled.

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/94f9e52671a173d49f685a142538641c471c53cb
https://hg.mozilla.org/mozilla-central/rev/94f9e52671a1

Comment 6

[Ryan VanderMeulen [:RyanVM]]

If I'm reading the blame correctly, this is a regression from bug 1629882? If so, do we need to backport this to ESR78 also?

Comment 7

[Yaron Tausky [:ytausky]]

It's not related, as far as I can tell. I think it was there all along until a tweet brought this to our attention. We should backport it, I'll check if it applies cleanly to ESR78.

Comment 8

[Tom Ritter [:tjr]]

Could you link to the tweet or the person who tweeted it for reference purposes?

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Tom Ritter [:tjr] (ni? for response to sec-[advisories/bounties/ratings/cves]) from comment #8)

Could you link to the tweet or the person who tweeted it for reference purposes?

https://twitter.com/SecurityMB/status/1321749513724289027

Comment 10

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 11

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 12

[Daniel Veditz [:dveditz]]

(In reply to Yaron Tausky [:ytausky] from comment #7)

It's not related, as far as I can tell. I think it was there all along until a tweet brought this to our attention. We should backport it, I'll check if it applies cleanly to ESR78.

What ever happened to this? Did it apply cleanly? Was it fixed? not fixed but should be? Turned out to not affect ESR78?

Comment 13

[Yaron Tausky [:ytausky]]

Apologies, I lost track of this. It applies cleanly, I'll request an uplift now.

Comment 14

[Yaron Tausky [:ytausky]]

Comment on attachment 9186227 [details]
Bug 1674343 - Check for secure context when deciding to intercept r=asuth

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: It's a simple patch, so there's not much risk involved.
• User impact if declined: I'm not sure if this is exploitable or not.
• Fix Landed on Version: 84
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It's low risk because it's a simple patch that's been verified on nightly already.
• String or UUID changes made by this patch:

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9186227 [details]
Bug 1674343 - Check for secure context when deciding to intercept r=asuth

Approved for 78.7esr.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/1e79801753dc

Comment 17

[Frederik Braun [:freddy]]

Attachment: advisory.txt

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b3bc56f4498
Test that service workers respect secure context restrictions r=asuth

Comment 19

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/32155 for changes under testing/web-platform/tests

Comment 20

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/7b3bc56f4498

Comment 21

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot