diff --git a/CefSharp.Core.Runtime/Internals/CefCertificateCallbackWrapper.h b/CefSharp.Core.Runtime/Internals/CefCertificateCallbackWrapper.h index 2fe2883b4..b7b3783fd 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 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: 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(); diff --git a/CefSharp.Core.Runtime/Internals/ClientAdapter.cpp b/CefSharp.Core.Runtime/Internals/ClientAdapter.cpp index 91808c0b4..7ef9f46fb 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,