-
Notifications
You must be signed in to change notification settings - Fork 15k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: cherry-pick 51daffbf5cd8 from chromium (#35548)
* chore: [19-x-y] cherry-pick 51daffbf5cd8 from chromium * chore: update patches Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Co-authored-by: Electron Bot <electron@github.com> Co-authored-by: Charles Kerr <charles@charleskerr.com>
- Loading branch information
1 parent
0e0b638
commit 14c64ed
Showing
2 changed files
with
46 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Yutaka Hirano <yhirano@chromium.org> | ||
Date: Mon, 4 Jul 2022 11:48:20 +0000 | ||
Subject: Fix UAF on network::URLLoader | ||
|
||
network::URLLoader::SetUpUpload calls NotifyCompleted asynchronously, | ||
as it can be called in the constructor and we don't want to run | ||
NotifyCompleted in the constructor. | ||
|
||
The problem is that it attaches a raw pointer to the method, which leads to a use-after-free problem if the URLLoader is destructed before | ||
NotifyCompleted is called. | ||
|
||
Use weak pointers instead of raw pointers to avoid the problem. | ||
|
||
Bug: 1340253 | ||
Change-Id: Iacb1e772bf7a8e3de4a7bb9de342fea9ba0f3f3c | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3740150 | ||
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org> | ||
Commit-Queue: Yutaka Hirano <yhirano@chromium.org> | ||
Cr-Commit-Position: refs/heads/main@{#1020539} | ||
|
||
diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc | ||
index dbe9e809898ee720c0670e9c6db73c38658613b2..666e82ee2f14a77592b52159ee81a2029de28172 100644 | ||
--- a/services/network/url_loader.cc | ||
+++ b/services/network/url_loader.cc | ||
@@ -793,8 +793,8 @@ void URLLoader::OpenFilesForUpload(const ResourceRequest& request) { | ||
// initializing before getting deleted. | ||
base::SequencedTaskRunnerHandle::Get()->PostTask( | ||
FROM_HERE, | ||
- base::BindOnce(&URLLoader::NotifyCompleted, base::Unretained(this), | ||
- net::ERR_ACCESS_DENIED)); | ||
+ base::BindOnce(&URLLoader::NotifyCompleted, | ||
+ weak_ptr_factory_.GetWeakPtr(), net::ERR_ACCESS_DENIED)); | ||
return; | ||
} | ||
url_request_->LogBlockedBy("Opening Files"); | ||
@@ -813,7 +813,7 @@ void URLLoader::SetUpUpload(const ResourceRequest& request, | ||
// initializing before getting deleted. | ||
base::SequencedTaskRunnerHandle::Get()->PostTask( | ||
FROM_HERE, base::BindOnce(&URLLoader::NotifyCompleted, | ||
- base::Unretained(this), error_code)); | ||
+ weak_ptr_factory_.GetWeakPtr(), error_code)); | ||
return; | ||
} | ||
scoped_refptr<base::SequencedTaskRunner> task_runner = |