Skip to content

Commit

Permalink
No need to call FilterURL on trustworthy data in RFHI::ShowContextMenu.
Browse files Browse the repository at this point in the history
Before this CL, RenderFrameHostImpl::ShowContextMenu would call
`process->FilterURL(...page_url)`.  This call was

1) Incorrect: using `this` frame's (potentially subframe's) process
   to filter the URL of the main frame (`page_url` comes from
   `GetMainFrame()->GetLastCommittedURL()` called a few lines above).

2) Unnecessary: `page_url` is trustworthy (i.e. populated on
   browser-side data, rather than based on
   UntrustworthyContextMenuParams).

After this CL, RenderFrameHostImpl::ShowContextMenu no longer calls
FilterURL on A) `page_url` and B) `frame_url` (both of these are
based on trustworthy data).

Bug: 1257907
Change-Id: I9ed7e0311af5287da668e07d582caa6445fc3f92
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3223130
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#931788}
  • Loading branch information
anforowicz authored and Chromium LUCI CQ committed Oct 15, 2021
1 parent dff10cb commit 64070c3
Showing 1 changed file with 4 additions and 5 deletions.
9 changes: 4 additions & 5 deletions content/browser/renderer_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6230,16 +6230,15 @@ void RenderFrameHostImpl::ShowContextMenu(
// directly, don't show them in the context menu.
ContextMenuParams validated_params(params);
validated_params.page_url = GetMainFrame()->GetLastCommittedURL();
if (GetParent()) // Only populate |frame_url| for subframes.
validated_params.frame_url = GetLastCommittedURL();
// Only populate |frame_url| if `this` is actually a subframe.
validated_params.frame_url = GetParent() ? GetLastCommittedURL() : GURL();

// We don't validate |unfiltered_link_url| so that this field can be used
// when users want to copy the original link URL.
// when users want to copy the original link URL. We also don't filter the
// URLs based on trustworthy data (e.g. `page_url` and `frame_url`).
RenderProcessHost* process = GetProcess();
process->FilterURL(true, &validated_params.link_url);
process->FilterURL(true, &validated_params.src_url);
process->FilterURL(false, &validated_params.page_url);
process->FilterURL(true, &validated_params.frame_url);

// It is necessary to transform the coordinates to account for nested
// RenderWidgetHosts, such as with out-of-process iframes.
Expand Down

0 comments on commit 64070c3

Please sign in to comment.