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

Restrict opaque-safelisted requesters set more? #4

Closed
annevk opened this issue Jan 5, 2021 · 4 comments · Fixed by #16
Closed

Restrict opaque-safelisted requesters set more? #4

annevk opened this issue Jan 5, 2021 · 4 comments · Fixed by #16

Comments

@annevk
Copy link
Owner

annevk commented Jan 5, 2021

As this is only needed for media, perhaps we should only allow a set contains check when the user agent does a range request that is not for the start of the file.

cc @jakearchibald

@annevk
Copy link
Owner Author

annevk commented Jan 13, 2021

@anforowicz you might have thoughts on this as well.

@anforowicz
Copy link
Collaborator

This seems like a nice-to-have:

  • Restricting the set-contains-check to 206 responses (or something similar) makes sense
  • OTOH, it seems like a defense-in-depth / I don't see immediate/direct security benefits of doing this (since earlier MIME-type-checks and/or sniffing should confirm that only audio/video URLs are in the set)

@annevk
Copy link
Owner Author

annevk commented Jan 14, 2021

Good point. So if we get a range request, I guess we want:

  1. Is it in the set?
  2. Does sniffing for media not return undefined?
  3. If neither are true, return false.

And if we handle range requests separately from normal requests, we can also return false for 206 responses to normal requests.

(I guess we might still need to sniff for media in response to normal requests. I don't know if media elements only perform range requests or not.)

@anforowicz
Copy link
Collaborator

This all sounds good to me, except one thing - I am not sure how "sniffing for media" would work for range requests for the middle of a file, because it seems that there is a risk that a JSON file or a PDF file might include a media-like-sniffing-signature somewhere inside. So, maybe we should restrict "sniffing for media" to the first bytes of a subresource (i.e. excluding responses carrying the middle of a resource)?

annevk added a commit that referenced this issue Jan 15, 2021
annevk added a commit that referenced this issue Oct 4, 2021
@annevk annevk closed this as completed in #16 Oct 7, 2021
annevk added a commit that referenced this issue Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants