Closed
Bug 1285476
Opened 8 years ago
Closed 7 years ago
AppCache can be used to bypass CORS
Categories
(Core :: Networking: HTTP, defect)
Core
Networking: HTTP
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: annevk, Assigned: mayhemer)
References
Details
(Whiteboard: [necko-active])
Attachments
(1 file, 1 obsolete file)
11.23 KB,
patch
|
jduell.mcbugs
:
review+
|
Details | Diff | Splinter Review |
See https://github.com/whatwg/html/issues/1427 for details and a potential solution (double caching). This is a security issue, but it's made public already.
Updated•8 years ago
|
Assignee: nobody → honzab.moz
Whiteboard: [necko-active]
![]() |
Assignee | |
Comment 1•8 years ago
|
||
The test case confirms we suffer from the issue. On reload of the evil site (from [1]) we load the anonymous xhr request from appcache. Appcache doesn't recognize the 'anonymous' flag. It means we serve non-anonymously loaded content to anonymous requests. That is bad. However, there used to be an issue from times we were honoring the anonymity flag - bug 687758. Honoring it will break some sites again, but I personally don't care at all. Appcache is a dead technology. [1] https://github.com/jakearchibald/appcache-credentials
Status: NEW → ASSIGNED
![]() |
Assignee | |
Comment 2•8 years ago
|
||
Attachment #8775149 -
Flags: review?(michal.novotny)
![]() |
Assignee | |
Comment 3•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a3f1c1fd561
Reporter | ||
Comment 4•8 years ago
|
||
Note that an alternative that should not break sites is double requests and double caching for each entry in AppCache. Not ideal obviously.
![]() |
Assignee | |
Comment 5•8 years ago
|
||
(In reply to Anne (:annevk) from comment #4) > Note that an alternative that should not break sites is double requests and > double caching for each entry in AppCache. Not ideal obviously. I know. But I don't want to allocate resources for that right now. OTOH, I'm spending recently quite a lot of time on appcache already, so that I may fix this along...
![]() |
Assignee | |
Comment 6•8 years ago
|
||
Mutually exclusive with the v1 patch. This is a roughly manually tested stuff.
Attachment #8775295 -
Flags: review?(michal.novotny)
![]() |
Assignee | |
Comment 7•7 years ago
|
||
Comment on attachment 8775149 [details] [diff] [review] v1 (hard-forbid serving from appcache to anon requests) Doesn't make sense to deal with this patch.
Attachment #8775149 -
Attachment is obsolete: true
Attachment #8775149 -
Flags: review?(michal.novotny)
![]() |
Assignee | |
Comment 8•7 years ago
|
||
Comment on attachment 8775295 [details] [diff] [review] v2 (double caching) https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7f78406a117 Michal, feel free to pass to Jason, he has some experience with the uriloader parts.
Comment 9•7 years ago
|
||
Comment on attachment 8775295 [details] [diff] [review] v2 (double caching) Jason will be better reviewer for this.
Attachment #8775295 -
Flags: review?(michal.novotny) → review?(jduell.mcbugs)
Comment 10•7 years ago
|
||
Comment on attachment 8775295 [details] [diff] [review] v2 (double caching) Review of attachment 8775295 [details] [diff] [review]: ----------------------------------------------------------------- > This is a roughly manually tested stuff. Patch makes sense to me. Can we test it any better manually before we ship it?
Attachment #8775295 -
Flags: review?(jduell.mcbugs) → review+
![]() |
Assignee | |
Comment 11•7 years ago
|
||
(In reply to Jason Duell [:jduell] (needinfo me) from comment #10) > Comment on attachment 8775295 [details] [diff] [review] > v2 (double caching) > > Review of attachment 8775295 [details] [diff] [review]: > ----------------------------------------------------------------- > > > This is a roughly manually tested stuff. > > Patch makes sense to me. Can we test it any better manually before we ship > it? Do you want me to block on this before do more testing here? I did of course a check this does what's expected and anon resources load and also that the test case from https://github.com/jakearchibald/appcache-credentials no longer show the symptoms. IMHO, this enough to go.
Flags: needinfo?(jduell.mcbugs)
Keywords: checkin-needed
Comment 12•7 years ago
|
||
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6e56fc8ee868 Let appcache double-cache resources that are cross origin, r=michal
Keywords: checkin-needed
Comment 13•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6e56fc8ee868
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in
before you can comment on or make changes to this bug.
Description
•