Skip to content

Commit

Permalink
Fix UaF in ui::DropTargetEvent::DropTargetEvent.
Browse files Browse the repository at this point in the history
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 9f4b576)

Bug: 1392661
Change-Id: I3c62a7473ef9b6cdd223f75fbda50671f539f9eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4070787
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: David Yeung <dayeung@chromium.org>
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: 27d3765-refs/heads/main@{#1058933}
  • Loading branch information
David Yeung authored and Chromium LUCI CQ committed Dec 8, 2022
1 parent e4b9523 commit eed5a4d
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 11 deletions.
20 changes: 10 additions & 10 deletions content/browser/web_contents/web_contents_view_aura.cc
Expand Up @@ -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();
}
Expand Down Expand Up @@ -1469,7 +1470,7 @@ void WebContentsViewAura::OnDragEntered(const ui::DropTargetEvent& event) {
}

void WebContentsViewAura::DragUpdatedCallback(
ui::DropTargetEvent event,
DropMetadata drop_metadata,
std::unique_ptr<DropData> drop_data,
base::WeakPtr<RenderWidgetHostViewBase> target,
absl::optional<gfx::PointF> transformed_pt) {
Expand All @@ -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<RenderWidgetHostViewBase*>(
web_contents_->GetRenderWidgetHostView())
->TransformPointToCoordSpaceForView(
event.location_f(),
drop_metadata.localized_location,
static_cast<RenderWidgetHostViewBase*>(
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);
}
Expand All @@ -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();
Expand All @@ -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()) {
Expand All @@ -1542,13 +1542,13 @@ aura::client::DragUpdateInfo WebContentsViewAura::OnDragUpdated(
std::unique_ptr<DropData> drop_data = std::make_unique<DropData>();
// 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<int>(current_drag_op_);
Expand Down
6 changes: 5 additions & 1 deletion content/browser/web_contents/web_contents_view_aura.h
Expand Up @@ -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;
Expand Down Expand Up @@ -265,7 +269,7 @@ class CONTENT_EXPORT WebContentsViewAura
std::unique_ptr<DropData> drop_data,
base::WeakPtr<RenderWidgetHostViewBase> target,
absl::optional<gfx::PointF> transformed_pt);
void DragUpdatedCallback(ui::DropTargetEvent event,
void DragUpdatedCallback(DropMetadata drop_metadata,
std::unique_ptr<DropData> drop_data,
base::WeakPtr<RenderWidgetHostViewBase> target,
absl::optional<gfx::PointF> transformed_pt);
Expand Down

0 comments on commit eed5a4d

Please sign in to comment.