From eed5a4de2c40ce0a35e6412177178b4ef22a7239 Mon Sep 17 00:00:00 2001 From: David Yeung Date: Thu, 8 Dec 2022 17:56:44 +0000 Subject: [PATCH] Fix UaF in ui::DropTargetEvent::DropTargetEvent. There is an async operation in WebContentsViewAura that uses a ui::DropTargetEvent. DropTargetEvent has a pointer to OSExchangeData which gets destroyed before the async operation is called. This triggers the UaF because the operation attempts to reference a freed object (OSExchangeData). Fix is for WebContentsViewAura::DragUpdatedCallback to use a DropMetadata struct instead of a ui::DropTargetEvent. This is the same pattern used by other callbacks in WebContentsViewAura. (cherry picked from commit 9f4b5761c546a118b7187c0c7ddcb9ee5756f32c) Bug: 1392661 Change-Id: I3c62a7473ef9b6cdd223f75fbda50671f539f9eb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4070787 Reviewed-by: Avi Drissman Commit-Queue: David Yeung Cr-Original-Commit-Position: refs/heads/main@{#1078218} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4085256 Cr-Commit-Position: refs/branch-heads/5359@{#1125} Cr-Branched-From: 27d3765d341b09369006d030f83f582a29eb57ae-refs/heads/main@{#1058933} --- .../web_contents/web_contents_view_aura.cc | 20 +++++++++---------- .../web_contents/web_contents_view_aura.h | 6 +++++- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/content/browser/web_contents/web_contents_view_aura.cc b/content/browser/web_contents/web_contents_view_aura.cc index 7a7f170f3cd9f..01e059f56861f 100644 --- a/content/browser/web_contents/web_contents_view_aura.cc +++ b/content/browser/web_contents/web_contents_view_aura.cc @@ -365,6 +365,7 @@ aura::Window* GetHostWindow(aura::Window* window) { WebContentsViewAura::DropMetadata::DropMetadata( const ui::DropTargetEvent& event) { localized_location = event.location_f(); + root_location = event.root_location_f(); source_operations = event.source_operations(); flags = event.flags(); } @@ -1469,7 +1470,7 @@ void WebContentsViewAura::OnDragEntered(const ui::DropTargetEvent& event) { } void WebContentsViewAura::DragUpdatedCallback( - ui::DropTargetEvent event, + DropMetadata drop_metadata, std::unique_ptr drop_data, base::WeakPtr target, absl::optional transformed_pt) { @@ -1490,24 +1491,23 @@ void WebContentsViewAura::DragUpdatedCallback( aura::Window* root_window = GetNativeView()->GetRootWindow(); aura::client::ScreenPositionClient* screen_position_client = aura::client::GetScreenPositionClient(root_window); - gfx::PointF screen_pt = event.root_location_f(); + gfx::PointF screen_pt = drop_metadata.root_location; if (screen_position_client) screen_position_client->ConvertPointToScreen(root_window, &screen_pt); if (target_rwh != current_rwh_for_drag_.get()) { if (current_rwh_for_drag_) { - gfx::PointF transformed_leave_point = event.location_f(); + gfx::PointF transformed_leave_point = drop_metadata.localized_location; static_cast( web_contents_->GetRenderWidgetHostView()) ->TransformPointToCoordSpaceForView( - event.location_f(), + drop_metadata.localized_location, static_cast( current_rwh_for_drag_->GetView()), &transformed_leave_point); current_rwh_for_drag_->DragTargetDragLeave(transformed_leave_point, screen_pt); } - DropMetadata drop_metadata(event); DragEnteredCallback(drop_metadata, std::move(drop_data), target, transformed_pt); } @@ -1518,10 +1518,11 @@ void WebContentsViewAura::DragUpdatedCallback( DCHECK(transformed_pt.has_value()); blink::DragOperationsMask op_mask = - ConvertToDragOperationsMask(event.source_operations()); + ConvertToDragOperationsMask(drop_metadata.source_operations); target_rwh->DragTargetDragOver( transformed_pt.value(), screen_pt, op_mask, - ui::EventFlagsToWebEventModifiers(event.flags()), base::DoNothing()); + ui::EventFlagsToWebEventModifiers(drop_metadata.flags), + base::DoNothing()); if (drag_dest_delegate_) drag_dest_delegate_->OnDragOver(); @@ -1531,7 +1532,6 @@ aura::client::DragUpdateInfo WebContentsViewAura::OnDragUpdated( const ui::DropTargetEvent& event) { if (web_contents_->ShouldIgnoreInputEvents()) return aura::client::DragUpdateInfo(); - aura::client::DragUpdateInfo drag_info; auto* focused_frame = web_contents_->GetFocusedFrame(); if (focused_frame && !web_contents_->GetBrowserContext()->IsOffTheRecord()) { @@ -1542,13 +1542,13 @@ aura::client::DragUpdateInfo WebContentsViewAura::OnDragUpdated( std::unique_ptr drop_data = std::make_unique(); // Calling this here as event.data might become invalid inside the callback. PrepareDropData(drop_data.get(), event.data()); - + DropMetadata drop_metadata(event); web_contents_->GetInputEventRouter() ->GetRenderWidgetHostAtPointAsynchronously( web_contents_->GetRenderViewHost()->GetWidget()->GetView(), event.location_f(), base::BindOnce(&WebContentsViewAura::DragUpdatedCallback, - weak_ptr_factory_.GetWeakPtr(), event, + weak_ptr_factory_.GetWeakPtr(), drop_metadata, std::move(drop_data))); drag_info.drag_operation = static_cast(current_drag_op_); diff --git a/content/browser/web_contents/web_contents_view_aura.h b/content/browser/web_contents/web_contents_view_aura.h index 3425436e8c1e9..45d6a8691e9a7 100644 --- a/content/browser/web_contents/web_contents_view_aura.h +++ b/content/browser/web_contents/web_contents_view_aura.h @@ -84,6 +84,10 @@ class CONTENT_EXPORT WebContentsViewAura // Location local to WebContentsViewAura. gfx::PointF localized_location; + + // Root location of the drop target event. + gfx::PointF root_location; + // The supported DnD operation of the source. A bitmask of // ui::mojom::DragOperations. int source_operations; @@ -265,7 +269,7 @@ class CONTENT_EXPORT WebContentsViewAura std::unique_ptr drop_data, base::WeakPtr target, absl::optional transformed_pt); - void DragUpdatedCallback(ui::DropTargetEvent event, + void DragUpdatedCallback(DropMetadata drop_metadata, std::unique_ptr drop_data, base::WeakPtr target, absl::optional transformed_pt);