Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

register() lacks same origin check #1518

Closed
annevk opened this issue Jun 15, 2020 · 9 comments
Closed

register() lacks same origin check #1518

annevk opened this issue Jun 15, 2020 · 9 comments
Assignees
Labels
security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response.

Comments

@annevk
Copy link
Member

annevk commented Jun 15, 2020

Am I missing something or does register() not perform a same origin check anywhere?

@annevk annevk added the security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response. label Jun 15, 2020
@mozfreddyb
Copy link

The "Start Register" algorithm has some checks in step 3 and 4:

  • step 3 checks that scheme is http or https
  • step 4 checks that the scriptURL's path doesn't contain "%2F" or "%5C".

The only invokation of Start Register is in ServiceWorkerContainer.register(), which does not apply any checks, just parsing in step 3.

P.S.: This issue filed was based on a question I had directed at @annevk based on folks requesting subresource integrity support for ServiceWorkers - which is providing most of its on cross-origin resources only.

@mkruisselbrink
Copy link
Collaborator

Start Register schedules a "Register" job, which ends up invoking https://w3c.github.io/ServiceWorker/#register-algorithm, which does do same origin checks in step 2 and 3 (the "referrer" is set to "client's creation URL" in the register() method.

This definitely could be cleaned up more to make it clearer. I remember there was a good reason for making this this complicated back when I spec'ed header-based installation, but that was a bad idea anyway. So probably it could be done much more straight forward.

@annevk
Copy link
Member Author

annevk commented Jun 15, 2020

Thanks, though note that it does not actually do a same origin check. It checks if A is B, not if A is same origin with B. Those are different checks.

@annevk
Copy link
Member Author

annevk commented Aug 31, 2020

What happens in case of a cross-origin redirect?

@wanderview
Copy link
Member

wanderview commented Aug 31, 2020

Per step 5 of "perform the fetch" here:

https://w3c.github.io/ServiceWorker/#update-algorithm

The request.redirect mode is set to "error".

@annevk
Copy link
Member Author

annevk commented Aug 31, 2020

I see, but it's weird that "perform the fetch" isn't passed to the algorithm (or is an official optional parameter). @domenic that might be worth cleaning up.

@mfalken
Copy link
Member

mfalken commented Jan 29, 2021

I moved this to whatwg/html#6339.

@mfalken mfalken closed this as completed Jan 29, 2021
@mfalken
Copy link
Member

mfalken commented Jan 29, 2021

I missed the comment here: #1518 (comment)

@mfalken mfalken reopened this Jan 29, 2021
@mfalken mfalken self-assigned this Jan 29, 2021
@mfalken
Copy link
Member

mfalken commented Mar 5, 2021

Fixed in #1568

@mfalken mfalken closed this as completed Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response.
Projects
None yet
Development

No branches or pull requests

5 participants