Bug 229120 - Allow a same-origin <iframe> to programmatically navigate its top frame to an external URL
Summary: Allow a same-origin <iframe> to programmatically navigate its top frame to an...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Shvayka
URL:
Keywords: InRadar
Depends on:
Blocks: 231702
  Show dependency tree / graph
 
Reported: 2021-08-15 08:17 PDT by Alexey Shvayka
Modified: 2022-10-10 14:09 PDT (History)
12 users (show)

See Also:

Attachments
Patch (31.41 KB, patch)
2021-08-15 08:21 PDT, Alexey Shvayka 		no flags Details | Formatted Diff | Diff
Patch (88.35 KB, patch)
2021-10-19 11:04 PDT, Alexey Shvayka 		no flags Details | Formatted Diff | Diff
Patch (92.30 KB, patch)
2021-10-19 14:11 PDT, Alexey Shvayka 		ggaren: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Shvayka 2021-08-15 08:17:09 PDT 
Allow a same-origin <iframe> to programmatically navigate its top frame to an external URL
Comment 1 Alexey Shvayka 2021-08-15 08:21:13 PDT 
Created attachment 435564 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2021-08-22 08:18:15 PDT 
<rdar://problem/82217976>
Comment 3 Geoffrey Garen 2021-10-12 15:00:39 PDT 
Comment on attachment 435564 [details]
Patch

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

I think the general approach to allow an iframe to open an external URL, if and only if it is same origin and a top-level navigation, seems sounds, and a good way to resolve the regression. It's sound because you're right, there was never any security boundary there anyway. And, of course, we want to resolve the regression.

> Source/WebCore/ChangeLog:20
> +        This patch reverts r280826 and makes it legal for iframes to open external URLs

makes it legal for same-origin iframes
Comment 4 Alexey Shvayka 2021-10-19 11:04:06 PDT 
Created attachment 441758 [details]
Patch

Same idea, way better implementation that enabled nice refactoring.
Comment 5 Alexey Shvayka 2021-10-19 14:11:42 PDT 
Created attachment 441796 [details]
Patch

Fix iOS build, ignore http/tests/navigation-policy on non-WK2, and fix isMainFrame() called on nullptr.
Comment 6 Geoffrey Garen 2022-01-12 11:11:29 PST 
Comment on attachment 441796 [details]
Patch

r=me

I recommend double-checking where the ShouldOpenExternalURLsPolicy originates in the navigation request, and verifying that it enforces a main frame requirement.
Comment 7 Ahmad Saleem 2022-10-10 14:09:24 PDT 
Checking via BugID, it seems this r+ patch didn't landed.

Is this needed now? Thanks!