diff --git a/chromium/content/browser/web_contents/web_contents_view_aura.cc b/chromium/content/browser/web_contents/web_contents_view_aura.cc index 01e059f5686..3e2c55d93dc 100644 --- a/chromium/content/browser/web_contents/web_contents_view_aura.cc +++ b/chromium/content/browser/web_contents/web_contents_view_aura.cc @@ -763,13 +763,10 @@ void WebContentsViewAura::PrepareDropData( // Do not add FileContents if this is a tainted-cross-origin same-page image // (https://crbug.com/1264873). bool access_allowed = - // Drag started outside blink. !drag_start_ || - // Drag began in blink, but image access is allowed. - drag_start_->image_accessible_from_frame || - // Drag began in blink, but in a different WebContents. - GetRenderViewHostID(web_contents_->GetRenderViewHost()) != - drag_start_->view_id; + // Drag began in this top-level WebContents, and image access is allowed + // (not cross-origin). + drag_start_->image_accessible_from_frame; data.GetFilenames(&drop_data->filenames); if (access_allowed && drop_data->filenames.empty()) { base::FilePath filename; @@ -885,6 +882,8 @@ bool WebContentsViewAura::IsValidDragTarget( // drags between cross-origin frames within the same page. Otherwise, a // malicious attacker could abuse drag interactions to leak information // across origins without explicit user intent. + // `drag_start_` is null when the drag started outside of the browser or from + // a different top-level WebContents. if (!drag_start_) return true; @@ -892,35 +891,19 @@ bool WebContentsViewAura::IsValidDragTarget( // perform the check unless it already has access to the starting // document's origin. If the SiteInstanceGroups match, then the process // allocation policy decided that it is OK for the source and target - // frames to live in the same renderer process. Furthermore, it means that - // either the source and target frame are part of the same `blink::Page` or - // that there is an opener relationship and would cross tab boundaries. Allow - // this drag to the renderer. Blink will perform an additional check against + // frames to live in the same renderer process. Furthermore, having matching + // SiteInstanceGroups means that either (1) the source and target frame are + // part of the same blink::Page, or (2) that they are in the same Browsing + // Context Group and the drag would cross tab boundaries (the latter of which + // can't happen here since `drag_start_` is null). Allow this drag to the + // renderer. Blink will perform an additional check against // `blink::DragController::drag_initiator_` to decide whether or not to // allow the drag operation. This can be done in the renderer, as the // browser-side checks only have local tree fragment (potentially with // multiple origins) granularity at best, but a drag operation eventually // targets one single frame in that local tree fragment. - bool same_site_instance_group = target_rwh->GetSiteInstanceGroup()->GetId() == - drag_start_->site_instance_group_id; - if (same_site_instance_group) - return true; - - // Otherwise, if the SiteInstanceGroups do not match, enforce explicit - // user intent by ensuring this drag operation is crossing page boundaries. - // `drag_start_->view_id` is set to the main `RenderFrameHost`'s - // `RenderViewHost`'s ID when a drag starts, so if the two IDs match here, - // the drag is within the same page and disallowed. - // - // Drags between an embedder and an inner `WebContents` will disallowed by - // the above view ID check because `WebContentsViewAura` is always created - // for the outermost view. Inner `WebContents` will have a - // `WebContentsViewChildFrame` so when dragging between an inner - // `WebContents` and its embedder the view IDs will be the same. - bool cross_tab_drag = - GetRenderViewHostID(web_contents_->GetRenderViewHost()) != - drag_start_->view_id; - return cross_tab_drag; + return target_rwh->GetSiteInstanceGroup()->GetId() == + drag_start_->site_instance_group_id; } //////////////////////////////////////////////////////////////////////////////// @@ -1175,7 +1158,6 @@ void WebContentsViewAura::StartDragging( drag_start_ = DragStart(source_rwh->GetSiteInstanceGroup()->GetId(), - GetRenderViewHostID(web_contents_->GetRenderViewHost()), drop_data.file_contents_image_accessible); ui::TouchSelectionController* selection_controller = GetSelectionController(); diff --git a/chromium/content/browser/web_contents/web_contents_view_aura.h b/chromium/content/browser/web_contents/web_contents_view_aura.h index 45d6a8691e9..b12a171b202 100644 --- a/chromium/content/browser/web_contents/web_contents_view_aura.h +++ b/chromium/content/browser/web_contents/web_contents_view_aura.h @@ -163,7 +163,7 @@ class CONTENT_EXPORT WebContentsViewAura // Returns whether |target_rwh| is a valid RenderWidgetHost to be dragging // over. This enforces that same-page, cross-site drags are not allowed. See - // crbug.com/666858. + // crbug.com/666858, crbug.com/1266953, crbug.com/1485266. bool IsValidDragTarget(RenderWidgetHostImpl* target_rwh) const; // Called from CreateView() to create |window_|. @@ -342,7 +342,7 @@ class CONTENT_EXPORT WebContentsViewAura std::unique_ptr window_observer_; // The WebContentsImpl whose contents we display. - raw_ptr web_contents_; + const raw_ptr web_contents_; std::unique_ptr delegate_; @@ -360,33 +360,21 @@ class CONTENT_EXPORT WebContentsViewAura // avoid sending the drag exited message after leaving the current view. GlobalRoutingID current_rvh_for_drag_; - // We track the IDs of the source RenderProcessHost and RenderViewHost from - // which the current drag originated. These are used to ensure that drag - // events do not fire over a cross-site frame (with respect to the source - // frame) in the same page (see crbug.com/666858). Specifically, the - // RenderViewHost is used to check the "same page" property, while the - // RenderProcessHost is used to check the "cross-site" property. Note that the - // reason the RenderProcessHost is tracked instead of the RenderWidgetHost is - // so that we still allow drags between non-contiguous same-site frames (such - // frames will have the same process, but different widgets). Note also that - // the RenderViewHost may not be in the same process as the RenderProcessHost, - // since the view corresponds to the page, while the process is specific to - // the frame from which the drag started. - // We also track whether a dragged image is accessible from its frame, so we - // can disallow tainted-cross-origin same-page drag-drop. + // Used to track security-salient details about a drag source. See + // documentation in `IsValidDragTarget()` for `site_instance_group_id`. + // See crbug.com/1264873 for `image_accessible_from_frame`. struct DragStart { DragStart(SiteInstanceGroupId site_instance_group_id, - GlobalRoutingID view_id, bool image_accessible_from_frame) : site_instance_group_id(site_instance_group_id), - view_id(view_id), image_accessible_from_frame(image_accessible_from_frame) {} ~DragStart() = default; SiteInstanceGroupId site_instance_group_id; - GlobalRoutingID view_id; bool image_accessible_from_frame; }; + // Will hold a value when the current drag started in this page (outermost + // WebContents). absl::optional drag_start_; // Responsible for handling gesture-nav and pull-to-refresh UI.