Bug 211092 - Web Inspector: Storage: can see third-party cookies
Summary: Web Inspector: Storage: can see third-party cookies
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-27 14:41 PDT by Devin Rousso
Modified: 2020-04-27 19:47 PDT (History)
11 users (show)

See Also:


Attachments
Patch (2.06 KB, patch)
2020-04-27 14:44 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (18.38 KB, patch)
2020-04-27 15:52 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2020-04-27 14:41:19 PDT
The early return if the given `WebFrame` is the main frame means that if a third-party resource is loaded into the main frame, and cookies are queried for that third-party's resource in the main frame, we will allow cookie access even though it's a third-party resource simply because it was loaded into the main frame.  This is incorrect, as we should always verify that the domain of the url being used to query for cookies matches the first-party domain before granting access.
Comment 1 Devin Rousso 2020-04-27 14:41:34 PDT
<rdar://problem/62469078>
Comment 2 Devin Rousso 2020-04-27 14:44:04 PDT
Created attachment 397741 [details]
Patch
Comment 3 Darin Adler 2020-04-27 14:46:54 PDT
No test?
Comment 4 Devin Rousso 2020-04-27 14:47:50 PDT
(In reply to Darin Adler from comment #3)
> No test?
I'm working on that now :)
Comment 5 Maciej Stachowiak 2020-04-27 14:48:34 PDT
Whether media elements get cookies in a third party context is probably tricky to test but definitely not impossible to test, and we certainly want to avoid similar bugs in the future.
Comment 6 Maciej Stachowiak 2020-04-27 14:48:57 PDT
Comment on attachment 397741 [details]
Patch

r- for lack of test for now
Comment 7 Chris Dumez 2020-04-27 15:03:52 PDT
(In reply to Maciej Stachowiak from comment #5)
> Whether media elements get cookies in a third party context is probably
> tricky to test but definitely not impossible to test, and we certainly want
> to avoid similar bugs in the future.

I added Jer & Eric for advice. If I remember correctly, this is used by AirPlay on macOS only (and Web Inspector but I guess we are more concerned about the media case).
Comment 8 Eric Carlson 2020-04-27 15:29:23 PDT
Comment on attachment 397741 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397741&action=review

> Source/WebKit/ChangeLog:9
> +        The early return if the given `WebFrame` is the main frame means that if a third-party

s/is the main frame/in the main frame/
Comment 9 Eric Carlson 2020-04-27 15:35:09 PDT
(In reply to Chris Dumez from comment #7)
> 
> I added Jer & Eric for advice. If I remember correctly, this is used by
> AirPlay on macOS only (and Web Inspector but I guess we are more concerned
> about the media case).

getRawCookies is used for media on iOS only, see MediaPlayerPrivateAVFoundationObjC::createAVAssetForURL.

MediaPlayerPrivateAVFoundationObjC ultimately gets the cookies from HTMLMediaElement::mediaPlayerGetRawCookies, so you could add something to Internals similar to what we did for response sources in Internals::mediaResponseSources.
Comment 10 Devin Rousso 2020-04-27 15:52:11 PDT
Created attachment 397755 [details]
Patch
Comment 11 Chris Dumez 2020-04-27 16:00:47 PDT
Comment on attachment 397755 [details]
Patch

r=me
Comment 12 EWS 2020-04-27 19:47:18 PDT
Committed r260807: <https://trac.webkit.org/changeset/260807>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 397755 [details].