From adcfd6375c1d87242570262246c1213e7633600f Mon Sep 17 00:00:00 2001 From: John Hodnik Date: Wed, 19 Aug 2026 14:20:37 -0400 Subject: [PATCH 1/2] Fix deferred client certificate selection reading freed memory CefCertificateCallbackWrapper held the offered certificate list as `const X509CertificateList&`, bound to a stack local built in ClientAdapter::OnSelectClientCertificate. Once that handler returned the list was destroyed, so calling Select() at any later point walked freed memory and threw inside the thumbprint-matching loop, taking the host process down with it. CEF explicitly permits answering later. cef_request_handler.h says to return true and call Select "either in this method or at a later time", so a wrapper that outlives the handler has to own the list it selects from. It now holds a heap-allocated copy, freed in the finalizer. A ref class cannot contain a std::vector by value, hence the pointer. Copying the vector copies the reference-counted CefX509Certificate pointers, and those references are what keep the certificates alive. This is the remaining half of #2948. The comment above the caller reads "Create a copy of the vector in an attempt to fix #2948", and the copy is indeed made - but it is then bound by reference, so it dies at the same instant the original would have. Co-Authored-By: Claude Opus 5 (1M context) --- .../Internals/CefCertificateCallbackWrapper.h | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/CefSharp.Core.Runtime/Internals/CefCertificateCallbackWrapper.h b/CefSharp.Core.Runtime/Internals/CefCertificateCallbackWrapper.h index 2fe2883b46..07d632166b 100644 --- a/CefSharp.Core.Runtime/Internals/CefCertificateCallbackWrapper.h +++ b/CefSharp.Core.Runtime/Internals/CefCertificateCallbackWrapper.h @@ -19,11 +19,19 @@ namespace CefSharp { private: MCefRefPtr _callback; - const CefRequestHandler::X509CertificateList& _certificateList; + // Owned copy of the certificates Chromium offered, not a reference to the caller's. + // ClientAdapter::OnSelectClientCertificate builds that list as a stack local, so a + // reference to it dangles the moment the handler returns. CEF permits calling Select + // "either in this method or at a later time", so a wrapper that outlives the handler + // has to own the list it selects from, or a deferred Select reads freed memory. + // A ref class cannot hold a std::vector by value, hence the pointer. Copying the + // vector copies the reference-counted CefX509Certificate pointers, and those + // references are what keep the certificates themselves alive. + CefRequestHandler::X509CertificateList* _certificateList; public: CefCertificateCallbackWrapper(CefRefPtr& callback, const CefRequestHandler::X509CertificateList& certificates) - : _callback(callback), _certificateList(certificates) + : _callback(callback), _certificateList(new CefRequestHandler::X509CertificateList(certificates)) { } @@ -31,6 +39,9 @@ namespace CefSharp !CefCertificateCallbackWrapper() { _callback = nullptr; + + delete _certificateList; + _certificateList = nullptr; } ~CefCertificateCallbackWrapper() @@ -53,8 +64,8 @@ namespace CefSharp auto certThumbprint = cert->Thumbprint; std::vector>::const_iterator it = - _certificateList.begin(); - for (; it != _certificateList.end(); ++it) + _certificateList->begin(); + for (; it != _certificateList->end(); ++it) { auto bytes((*it)->GetDEREncoded()); auto byteSize = bytes->GetSize(); From 4979b64d7b2b6cdbbe5ecfe67fd3942863f749fb Mon Sep 17 00:00:00 2001 From: John Hodnik Date: Fri, 28 Aug 2026 21:24:53 -0400 Subject: [PATCH 2/2] Remove the redundant certificate vector copy in ClientAdapter The wrapper takes its own copy in its constructor, so the local copy added by a51cdd37 in ClientAdapter::OnSelectClientCertificate is now pure redundancy - two copies where one is needed. Pass `certificates` straight through instead. Also refreshes the comment on _certificateList, which described the caller's stack local that this removes. Co-Authored-By: Claude Opus 5 (1M context) --- .../Internals/CefCertificateCallbackWrapper.h | 16 ++++++++-------- .../Internals/ClientAdapter.cpp | 8 +++----- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/CefSharp.Core.Runtime/Internals/CefCertificateCallbackWrapper.h b/CefSharp.Core.Runtime/Internals/CefCertificateCallbackWrapper.h index 07d632166b..b7b3783fdd 100644 --- a/CefSharp.Core.Runtime/Internals/CefCertificateCallbackWrapper.h +++ b/CefSharp.Core.Runtime/Internals/CefCertificateCallbackWrapper.h @@ -19,14 +19,14 @@ namespace CefSharp { private: MCefRefPtr _callback; - // Owned copy of the certificates Chromium offered, not a reference to the caller's. - // ClientAdapter::OnSelectClientCertificate builds that list as a stack local, so a - // reference to it dangles the moment the handler returns. CEF permits calling Select - // "either in this method or at a later time", so a wrapper that outlives the handler - // has to own the list it selects from, or a deferred Select reads freed memory. - // A ref class cannot hold a std::vector by value, hence the pointer. Copying the - // vector copies the reference-counted CefX509Certificate pointers, and those - // references are what keep the certificates themselves alive. + // Owned copy of the certificates Chromium offered, not a reference to the caller's list. + // That list belongs to CEF for the duration of ClientAdapter::OnSelectClientCertificate, + // and this wrapper deliberately outlives that call - CEF allows Select to be called + // "either in this method or at a later time" - so a reference would dangle the moment + // the handler returns and a deferred Select would read freed memory. + // A ref class cannot hold a std::vector by value, hence the pointer. Copying the vector + // copies the reference-counted CefX509Certificate pointers, and those references are + // what keep the certificates themselves alive. CefRequestHandler::X509CertificateList* _certificateList; public: diff --git a/CefSharp.Core.Runtime/Internals/ClientAdapter.cpp b/CefSharp.Core.Runtime/Internals/ClientAdapter.cpp index 91808c0b4f..7ef9f46fb5 100644 --- a/CefSharp.Core.Runtime/Internals/ClientAdapter.cpp +++ b/CefSharp.Core.Runtime/Internals/ClientAdapter.cpp @@ -764,8 +764,6 @@ namespace CefSharp auto browserWrapper = GetBrowserWrapper(browser->GetIdentifier(), browser->IsPopup()); auto list = gcnew X509Certificate2Collection(); - // Create a copy of the vector in an attempt to fix #2948 - CefRequestHandler::X509CertificateList certs; std::vector >::const_iterator it = certificates.begin(); @@ -780,11 +778,11 @@ namespace CefSharp bytes->GetData(static_cast(src), byteSize, 0); auto cert = gcnew X509Certificate2(bufferByte); list->Add(cert); - - certs.push_back(*it); } - auto callbackWrapper = gcnew CefCertificateCallbackWrapper(callback, certs); + // Passed straight through. The wrapper takes its own reference to each certificate, so + // there is no need to copy the vector here. + auto callbackWrapper = gcnew CefCertificateCallbackWrapper(callback, certificates); return handler->OnSelectClientCertificate( _browserControl, browserWrapper, isProxy,