Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: prevent UAF crash in setCertificateVerifyProc #33254

Merged
merged 2 commits into from Mar 16, 2022
Merged

fix: prevent UAF crash in setCertificateVerifyProc #33254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e00f055
fix: prevent UAF crash in setCertificateVerifyProc
nornagon Mar 11, 2022
1fd5393
chore: update patches
patchup[bot] Mar 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions patches/chromium/expose_setuseragent_on_networkcontext.patch
View file
Open in desktop
Expand Up @@ -33,10 +33,10 @@ index 14c71cc69388da46f62d9835e2a06fef0870da02..9481ea08401ae29ae9c1d960491b05b3

} // namespace net
diff --git a/services/network/network_context.cc b/services/network/network_context.cc
index 326833bcf62ac0c0551d2c3664624333c8754e62..79a76cfa3c6eac5b06b40db05a16e3e77450d89a 100644
index b8a81db86d15d6e81de48526777a6e7708cbd7c6..3d2da40718acaeea5b0232ba93ad08c525626a55 100644
--- a/services/network/network_context.cc
+++ b/services/network/network_context.cc
@@ -1325,6 +1325,13 @@ void NetworkContext::SetNetworkConditions(
@@ -1337,6 +1337,13 @@ void NetworkContext::SetNetworkConditions(
std::move(network_conditions));
}

Expand Down
80 changes: 46 additions & 34 deletions patches/chromium/network_service_allow_remote_certificate_verification_logic.patch
View file
Open in desktop
Expand Up @@ -7,7 +7,7 @@ This adds a callback from the network service that's used to implement
session.setCertificateVerifyCallback.

diff --git a/services/network/network_context.cc b/services/network/network_context.cc
index 4a3f4fe1fb97a5715894ff236c5fa2f2684b728d..326833bcf62ac0c0551d2c3664624333c8754e62 100644
index 4a3f4fe1fb97a5715894ff236c5fa2f2684b728d..b8a81db86d15d6e81de48526777a6e7708cbd7c6 100644
--- a/services/network/network_context.cc
+++ b/services/network/network_context.cc
@@ -119,6 +119,11 @@
Expand All @@ -22,12 +22,38 @@ index 4a3f4fe1fb97a5715894ff236c5fa2f2684b728d..326833bcf62ac0c0551d2c3664624333
#if BUILDFLAG(IS_CT_SUPPORTED)
#include "components/certificate_transparency/chrome_ct_policy_enforcer.h"
#include "components/certificate_transparency/chrome_require_ct_delegate.h"
@@ -433,6 +438,79 @@ bool GetFullDataFilePath(
@@ -433,6 +438,91 @@ bool GetFullDataFilePath(

} // namespace

+class RemoteCertVerifier : public net::CertVerifier {
+ public:
+ class Request : public net::CertVerifier::Request {
+ public:
+ Request() {}
+ ~Request() override = default;
+ void OnRemoteResponse(
+ const RequestParams& params,
+ net::CertVerifyResult* verify_result,
+ int error_from_upstream,
+ net::CompletionOnceCallback callback,
+ int error_from_client,
+ const net::CertVerifyResult& verify_result_from_client) {
+ if (error_from_client == net::ERR_ABORTED) {
+ // use the default
+ std::move(callback).Run(error_from_upstream);
+ } else {
+ // use the override
+ verify_result->Reset();
+ verify_result->verified_cert = verify_result_from_client.verified_cert;
+ std::move(callback).Run(error_from_client);
+ }
+ }
+ base::WeakPtr<Request> GetWeakPtr() { return weak_factory_.GetWeakPtr(); }
+ private:
+ base::WeakPtrFactory<Request> weak_factory_{this};
+ };
+
+ RemoteCertVerifier(std::unique_ptr<net::CertVerifier> upstream): upstream_(std::move(upstream)) {
+ }
+ ~RemoteCertVerifier() override = default;
Expand All @@ -44,56 +70,42 @@ index 4a3f4fe1fb97a5715894ff236c5fa2f2684b728d..326833bcf62ac0c0551d2c3664624333
+ int Verify(const RequestParams& params,
+ net::CertVerifyResult* verify_result,
+ net::CompletionOnceCallback callback,
+ std::unique_ptr<Request>* out_req,
+ std::unique_ptr<CertVerifier::Request>* out_req,
+ const net::NetLogWithSource& net_log) override {
+ out_req->reset();
+
+ net::CompletionOnceCallback callback2 = base::BindOnce(
+ &RemoteCertVerifier::OnRequestFinished, base::Unretained(this),
+ params, std::move(callback), verify_result);
+ int result = upstream_->Verify(params, verify_result,
+ std::move(callback2), out_req, net_log);
+ if (result != net::ERR_IO_PENDING) {
+ // Synchronous completion
+ }
+
+ return result;
+ params, std::move(callback), verify_result, out_req);
+ return upstream_->Verify(params, verify_result, std::move(callback2), out_req, net_log);
+ }
+
+
+ void SetConfig(const Config& config) override {
+ upstream_->SetConfig(config);
+ }
+
+ void OnRequestFinished(const RequestParams& params, net::CompletionOnceCallback callback, net::CertVerifyResult* verify_result, int error) {
+ void OnRequestFinished(const RequestParams& params,
+ net::CompletionOnceCallback callback,
+ net::CertVerifyResult* verify_result,
+ std::unique_ptr<CertVerifier::Request>* out_req,
+ int error) {
+ if (client_.is_bound()) {
+ // We take a weak pointer to the request because deletion of the request
+ // is what signals cancellation. Thus if the request is cancelled, the
+ // callback won't be called, thus avoiding UAF, because |verify_result|
+ // is freed when the request is cancelled.
+ *out_req = std::make_unique<Request>();
+ base::WeakPtr<Request> weak_req = static_cast<Request*>(out_req->get())->GetWeakPtr();
+ client_->Verify(error, *verify_result, params.certificate(),
+ params.hostname(), params.flags(), params.ocsp_response(),
+ base::BindOnce(&RemoteCertVerifier::OnRemoteResponse,
+ base::Unretained(this), params, verify_result, error,
+ std::move(callback)));
+ base::BindOnce(&Request::OnRemoteResponse,
+ weak_req, params, verify_result, error, std::move(callback)));
+ } else {
+ std::move(callback).Run(error);
+ }
+ }
+
+ void OnRemoteResponse(
+ const RequestParams& params,
+ net::CertVerifyResult* verify_result,
+ int error,
+ net::CompletionOnceCallback callback,
+ int error2,
+ const net::CertVerifyResult& verify_result2) {
+ if (error2 == net::ERR_ABORTED) {
+ // use the default
+ std::move(callback).Run(error);
+ } else {
+ // use the override
+ verify_result->Reset();
+ verify_result->verified_cert = verify_result2.verified_cert;
+ std::move(callback).Run(error2);
+ }
+ }
+ private:
+ std::unique_ptr<net::CertVerifier> upstream_;
+ mojo::Remote<mojom::CertVerifierClient> client_;
Expand All @@ -102,7 +114,7 @@ index 4a3f4fe1fb97a5715894ff236c5fa2f2684b728d..326833bcf62ac0c0551d2c3664624333
constexpr uint32_t NetworkContext::kMaxOutstandingRequestsPerProcess;

NetworkContext::PendingCertVerify::PendingCertVerify() = default;
@@ -659,6 +737,13 @@ void NetworkContext::SetClient(
@@ -659,6 +749,13 @@ void NetworkContext::SetClient(
client_.Bind(std::move(client));
}

Expand All @@ -116,7 +128,7 @@ index 4a3f4fe1fb97a5715894ff236c5fa2f2684b728d..326833bcf62ac0c0551d2c3664624333
void NetworkContext::CreateURLLoaderFactory(
mojo::PendingReceiver<mojom::URLLoaderFactory> receiver,
mojom::URLLoaderFactoryParamsPtr params) {
@@ -2135,6 +2220,9 @@ URLRequestContextOwner NetworkContext::MakeURLRequestContext(
@@ -2135,6 +2232,9 @@ URLRequestContextOwner NetworkContext::MakeURLRequestContext(
std::move(cert_verifier));
cert_verifier = base::WrapUnique(cert_verifier_with_trust_anchors_);
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
Expand Down