Open Bug 1430999 Opened 6 years ago Updated 2 years ago

Service Workers can navigate uncontrolled clients

Categories

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

59 Branch
defect

Tracking

()

UNCONFIRMED

People

(Reporter: d.huigens, Unassigned)

References

(Blocks 1 open bug)

Details

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20180116220110

Steps to reproduce:

1. Register a Service Worker
2. Update the Service Worker
3. In the install event of the new (not-yet-controlling) Service Worker, attempt to navigate the client

Test case: open https://sw-navigate-test.glitch.me. It installs a SW which navigates to /test. After a few seconds, manually navigate back to / and it will serve a new SW. Observe that it navigates to /test a second time, while it shouldn't. In the console, you can see that the new SW is indeed not controlling any clients, e.g.:

[sw 2] [install event] controlled clients: 
Array []
[sw 2] [install event] all clients: 
Array [ "[object WindowClient]", "[object WindowClient]" ]


Actual results:

The client is navigated.


Expected results:

AFAIU, this shouldn't be allowed. Step 4 of https://w3c.github.io/ServiceWorker/#client-navigate says:

> If the context object’s associated service worker client's active service worker is not the context object’s relevant global object’s service worker, return a promise rejected with a TypeError.

This doesn't seem to be happening. In Chrome, it behaves as expected.
I opened a spec issue about this before:

https://github.com/w3c/ServiceWorker/issues/1254

The restriction doesn't make any sense to me because:

1. The client must be same-origin to the service worker already.  So its navigating your own window.
2. The service worker can postMessage() the window and then the window can set self.location to navigate.
3. The service worker could claim() many uncontrolled windows and then navigate them.

I think this check is a holdover in the spec from before it was possible to query uncontrolled clients.
I see. The reason this is an issue for me is the following: I'm trying to implement code signing of web applications using Service Workers. Basically, the SW checks all responses from the server, and also the new SW file when an updatefound event is fired. Under this security model, when the new SW runs, it is *not yet trusted* (until the old SW asynchronously checks it).

2. This can be prevented simply by not trusting postMessage()s from the new SW
3. This is far trickier to prevent, but also possible: if the old SW calls waitUntil() from a fetch event (one should have fired not long before the updatefound event), it delays the activation of the new SW if it calls skipWaiting(). It then can't claim() clients. (Originally, I thought it would be necessary to add a waitUntil() to updatefound events [0], until I thought of the workaround using a fetch event.)

One saving grace (for my use case) is that, even if the new SW navigates the client, the *old SW still controls it* (and can thus respond with whatever it wants). However, it's still a potential security issue if the user expects a tab to contain a signed web app from origin A, and an unsigned SW from origin A navigates the tab to origin B. (Especially since the old signed SW no longer has a client to show the user a warning in, although admittedly that can happen in other situations too.)

(If you prefer I respond on the spec issue instead, that's fine too.)

[0]: https://github.com/w3c/ServiceWorker/issues/1208
Please comment on the spec issue.  I am sympathetic to your effort, but I still feel you are trying to do something that doesn't fit the service worker model.  It has never been designed around the concept of an untrusted service worker script.
Priority: -- → P5
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.