qt: Avoid OpenSSL certstore-related memory leak

- Correctly manage the X509 and X509_STORE objects lifetime.
This commit is contained in:
Wladimir J. van der Laan 2016-11-19 16:12:23 +01:00
parent 5204598f8d
commit ed998ea7a0
2 changed files with 25 additions and 23 deletions

View File

@ -58,14 +58,19 @@ const char* BIP71_MIMETYPE_PAYMENTREQUEST = "application/bitcoin-paymentrequest"
// BIP70 max payment request size in bytes (DoS protection) // BIP70 max payment request size in bytes (DoS protection)
const qint64 BIP70_MAX_PAYMENTREQUEST_SIZE = 50000; const qint64 BIP70_MAX_PAYMENTREQUEST_SIZE = 50000;
X509_STORE* PaymentServer::certStore = NULL; struct X509StoreDeleter {
void PaymentServer::freeCertStore() void operator()(X509_STORE* b) {
{ X509_STORE_free(b);
if (PaymentServer::certStore != NULL)
{
X509_STORE_free(PaymentServer::certStore);
PaymentServer::certStore = NULL;
} }
};
struct X509Deleter {
void operator()(X509* b) { X509_free(b); }
};
namespace // Anon namespace
{
std::unique_ptr<X509_STORE, X509StoreDeleter> certStore;
} }
// //
@ -107,20 +112,15 @@ static void ReportInvalidCertificate(const QSslCertificate& cert)
// //
void PaymentServer::LoadRootCAs(X509_STORE* _store) void PaymentServer::LoadRootCAs(X509_STORE* _store)
{ {
if (PaymentServer::certStore == NULL)
atexit(PaymentServer::freeCertStore);
else
freeCertStore();
// Unit tests mostly use this, to pass in fake root CAs: // Unit tests mostly use this, to pass in fake root CAs:
if (_store) if (_store)
{ {
PaymentServer::certStore = _store; certStore.reset(_store);
return; return;
} }
// Normal execution, use either -rootcertificates or system certs: // Normal execution, use either -rootcertificates or system certs:
PaymentServer::certStore = X509_STORE_new(); certStore.reset(X509_STORE_new());
// Note: use "-system-" default here so that users can pass -rootcertificates="" // Note: use "-system-" default here so that users can pass -rootcertificates=""
// and get 'I don't like X.509 certificates, don't trust anybody' behavior: // and get 'I don't like X.509 certificates, don't trust anybody' behavior:
@ -167,11 +167,11 @@ void PaymentServer::LoadRootCAs(X509_STORE* _store)
QByteArray certData = cert.toDer(); QByteArray certData = cert.toDer();
const unsigned char *data = (const unsigned char *)certData.data(); const unsigned char *data = (const unsigned char *)certData.data();
X509* x509 = d2i_X509(0, &data, certData.size()); std::unique_ptr<X509, X509Deleter> x509(d2i_X509(0, &data, certData.size()));
if (x509 && X509_STORE_add_cert(PaymentServer::certStore, x509)) if (x509 && X509_STORE_add_cert(certStore.get(), x509.get()))
{ {
// Note: X509_STORE_free will free the X509* objects when // Note: X509_STORE increases the reference count to the X509 object,
// the PaymentServer is destroyed // we still have to release our reference to it.
++nRootCerts; ++nRootCerts;
} }
else else
@ -550,7 +550,7 @@ bool PaymentServer::processPaymentRequest(const PaymentRequestPlus& request, Sen
recipient.paymentRequest = request; recipient.paymentRequest = request;
recipient.message = GUIUtil::HtmlEscape(request.getDetails().memo()); recipient.message = GUIUtil::HtmlEscape(request.getDetails().memo());
request.getMerchant(PaymentServer::certStore, recipient.authenticatedMerchant); request.getMerchant(certStore.get(), recipient.authenticatedMerchant);
QList<std::pair<CScript, CAmount> > sendingTos = request.getPayTo(); QList<std::pair<CScript, CAmount> > sendingTos = request.getPayTo();
QStringList addresses; QStringList addresses;
@ -807,3 +807,8 @@ bool PaymentServer::verifyAmount(const CAmount& requestAmount)
} }
return fVerified; return fVerified;
} }
X509_STORE* PaymentServer::getCertStore()
{
return certStore.get();
}

View File

@ -83,7 +83,7 @@ public:
static void LoadRootCAs(X509_STORE* store = NULL); static void LoadRootCAs(X509_STORE* store = NULL);
// Return certificate store // Return certificate store
static X509_STORE* getCertStore() { return certStore; } static X509_STORE* getCertStore();
// OptionsModel is used for getting proxy settings and display unit // OptionsModel is used for getting proxy settings and display unit
void setOptionsModel(OptionsModel *optionsModel); void setOptionsModel(OptionsModel *optionsModel);
@ -140,9 +140,6 @@ private:
bool saveURIs; // true during startup bool saveURIs; // true during startup
QLocalServer* uriServer; QLocalServer* uriServer;
static X509_STORE* certStore; // Trusted root certificates
static void freeCertStore();
QNetworkAccessManager* netManager; // Used to fetch payment requests QNetworkAccessManager* netManager; // Used to fetch payment requests
OptionsModel *optionsModel; OptionsModel *optionsModel;