Summary: | Plug-ins from third-party domains in the top frame can still set LSOs | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Vicki Pfau <jeffrey+webkit> | ||||
Component: | WebKit Misc. | Assignee: | Vicki Pfau <jeffrey+webkit> | ||||
Status: | NEW --- | ||||||
Severity: | Normal | CC: | mjs | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Vicki Pfau
2013-02-25 18:14:08 PST
Created attachment 190179 [details]
Patch
That's worrisome. How bad do these sites break? We should probably discuss this offline. Comment on attachment 190179 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190179&action=review Code seems OK, but a bit confusing. > Source/WebCore/ChangeLog:15 > + (WebCore): Please remove bogus change log lines. > Source/WebCore/ChangeLog:17 > + (SecurityOrigin): Please remove bogus change log lines. > Source/WebCore/page/SecurityOrigin.cpp:386 > +bool SecurityOrigin::canAccessPluginStorage(const WebCore::KURL &url) const The prefix and formatting here is wrong. Should be: const KURL& url > Source/WebCore/page/SecurityOrigin.cpp:395 > + if (!url.isValid()) > + return true; Seems strange that the create function doesn’t do this check. > Source/WebCore/page/SecurityOrigin.cpp:398 > + RefPtr<SecurityOrigin> origin = create(url); > + return canAccessStorage(origin.get()); No need for the local variable. Reads better as: return canAccessStorage(create(url).get()); Also, wouldn’t it be better to call canAccessPuginStorage here instead of canAccessStorage? > Source/WebCore/page/SecurityOrigin.h:153 > + bool canAccessPluginStorage(const KURL&) const; To me it seems like this argument needs a name. Not at all clear what this URL represents. > LayoutTests/http/tests/security/cross-origin-same-frame-plugin-expected.txt:2 > +CONSOLE MESSAGE: line 11: false > +CONSOLE MESSAGE: line 12: false These sure are cryptic. Could you improve the test to make it clearer what these are about? > LayoutTests/platform/mac-wk2/http/tests/security/cross-origin-same-frame-plugin-expected.txt:2 > +CONSOLE MESSAGE: line 11: true > +CONSOLE MESSAGE: line 12: false These sure are cryptic. Could you improve the test to make it clearer what these are about? |