From 26b59d312375ad6391228308aabe45f0a85a1194 Mon Sep 17 00:00:00 2001 From: sudden6 Date: Thu, 21 Jun 2018 15:48:59 +0200 Subject: [PATCH] fix(Core): fix use after free of proxyAddrData --- src/core/core.cpp | 86 +++++++++++++++++++++++++++++++---------------- 1 file changed, 57 insertions(+), 29 deletions(-) diff --git a/src/core/core.cpp b/src/core/core.cpp index f82a5d2e2..99fceeb17 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -115,16 +115,45 @@ CoreAV* Core::getAv() namespace { -class ToxOptionsDeleter +/** + * @brief The ToxOptionsWrapper class wraps the Tox_Options struct and the matching + * proxy address data. This is needed to ensure both have equal lifetime and + * are correctly deleted. + */ +class ToxOptionsWrapper { public: - void operator()(Tox_Options* options) - { - tox_options_free(options); + ToxOptionsWrapper(Tox_Options *options, const QByteArray& proxyAddrData) { + this->options = options; + this->proxyAddrData = new QByteArray(proxyAddrData); } -}; -using ToxOptionsPtr = std::unique_ptr; + ~ToxOptionsWrapper() { + tox_options_free(options); + options = nullptr; + delete proxyAddrData; + proxyAddrData = nullptr; + } + + ToxOptionsWrapper (ToxOptionsWrapper && from) { + this->options = from.options; + from.options = nullptr; + this->proxyAddrData = from.proxyAddrData; + from.proxyAddrData = nullptr; + } + + operator Tox_Options* () { + return options; + } + + const char* getProxyAddrData() { + return this->proxyAddrData->data(); + } + +private: + Tox_Options *options = nullptr; + QByteArray *proxyAddrData = nullptr; +}; /** * @brief Map TOX_LOG_LEVEL to a string @@ -179,9 +208,9 @@ void onLogMessage(Tox *tox, TOX_LOG_LEVEL level, const char *file, uint32_t line /** * @brief Initializes Tox_Options instance * @param savedata Previously saved Tox data - * @return Tox_Options instance needed to create Tox instance + * @return ToxOptionsWrapper instance initialized to create Tox instance */ -ToxOptionsPtr initToxOptions(const QByteArray& savedata, const ICoreSettings* s) +ToxOptionsWrapper initToxOptions(const QByteArray& savedata, const ICoreSettings* s) { // IPv6 needed for LAN discovery, but can crash some weird routers. On by default, can be // disabled in options. @@ -192,7 +221,6 @@ ToxOptionsPtr initToxOptions(const QByteArray& savedata, const ICoreSettings* s) ICoreSettings::ProxyType proxyType = s->getProxyType(); quint16 proxyPort = s->getProxyPort(); QString proxyAddr = s->getProxyAddr(); - QByteArray proxyAddrData = proxyAddr.toUtf8(); if (!enableLanDiscovery) { qWarning() << "Core starting without LAN discovery. Peers can only be found through DHT."; @@ -203,22 +231,22 @@ ToxOptionsPtr initToxOptions(const QByteArray& savedata, const ICoreSettings* s) qWarning() << "Core starting with IPv6 disabled. LAN discovery may not work properly."; } - ToxOptionsPtr toxOptions = ToxOptionsPtr(tox_options_new(nullptr)); + ToxOptionsWrapper toxOptions = ToxOptionsWrapper(tox_options_new(nullptr), proxyAddr.toUtf8()); // register log first, to get messages as early as possible - tox_options_set_log_callback(toxOptions.get(), onLogMessage); + tox_options_set_log_callback(toxOptions, onLogMessage); - tox_options_set_ipv6_enabled(toxOptions.get(), enableIPv6); - tox_options_set_udp_enabled(toxOptions.get(), !forceTCP); - tox_options_set_local_discovery_enabled(toxOptions.get(), enableLanDiscovery); - tox_options_set_start_port(toxOptions.get(), 0); - tox_options_set_end_port(toxOptions.get(), 0); + tox_options_set_ipv6_enabled(toxOptions, enableIPv6); + tox_options_set_udp_enabled(toxOptions, !forceTCP); + tox_options_set_local_discovery_enabled(toxOptions, enableLanDiscovery); + tox_options_set_start_port(toxOptions, 0); + tox_options_set_end_port(toxOptions, 0); // No proxy by default - tox_options_set_proxy_type(toxOptions.get(), TOX_PROXY_TYPE_NONE); - tox_options_set_proxy_host(toxOptions.get(), nullptr); - tox_options_set_proxy_port(toxOptions.get(), 0); - tox_options_set_savedata_type(toxOptions.get(), !savedata.isNull() ? TOX_SAVEDATA_TYPE_TOX_SAVE : TOX_SAVEDATA_TYPE_NONE); - tox_options_set_savedata_data(toxOptions.get(), reinterpret_cast(savedata.data()), savedata.size()); + tox_options_set_proxy_type(toxOptions, TOX_PROXY_TYPE_NONE); + tox_options_set_proxy_host(toxOptions, nullptr); + tox_options_set_proxy_port(toxOptions, 0); + tox_options_set_savedata_type(toxOptions, !savedata.isNull() ? TOX_SAVEDATA_TYPE_TOX_SAVE : TOX_SAVEDATA_TYPE_NONE); + tox_options_set_savedata_data(toxOptions, reinterpret_cast(savedata.data()), savedata.size()); if (proxyType != ICoreSettings::ProxyType::ptNone) { if (proxyAddr.length() > MAX_PROXY_ADDRESS_LENGTH) { @@ -227,13 +255,13 @@ ToxOptionsPtr initToxOptions(const QByteArray& savedata, const ICoreSettings* s) qDebug() << "using proxy" << proxyAddr << ":" << proxyPort; // protection against changings in TOX_PROXY_TYPE enum if (proxyType == ICoreSettings::ProxyType::ptSOCKS5) { - tox_options_set_proxy_type(toxOptions.get(), TOX_PROXY_TYPE_SOCKS5); + tox_options_set_proxy_type(toxOptions, TOX_PROXY_TYPE_SOCKS5); } else if (proxyType == ICoreSettings::ProxyType::ptHTTP) { - tox_options_set_proxy_type(toxOptions.get(), TOX_PROXY_TYPE_HTTP); + tox_options_set_proxy_type(toxOptions, TOX_PROXY_TYPE_HTTP); } - tox_options_set_proxy_host(toxOptions.get(), proxyAddrData.data()); - tox_options_set_proxy_port(toxOptions.get(), proxyPort); + tox_options_set_proxy_host(toxOptions, toxOptions.getProxyAddrData()); + tox_options_set_proxy_port(toxOptions, proxyPort); } } @@ -248,7 +276,7 @@ ToxOptionsPtr initToxOptions(const QByteArray& savedata, const ICoreSettings* s) */ void Core::makeTox(QByteArray savedata) { - ToxOptionsPtr toxOptions = initToxOptions(savedata, s); + ToxOptionsWrapper toxOptions = initToxOptions(savedata, s); if (toxOptions == nullptr) { qCritical() << "could not allocate Tox Options data structure"; emit failedToStart(); @@ -256,7 +284,7 @@ void Core::makeTox(QByteArray savedata) } TOX_ERR_NEW tox_err; - tox = tox_new(toxOptions.get(), &tox_err); + tox = tox_new(toxOptions, &tox_err); switch (tox_err) { case TOX_ERR_NEW_OK: @@ -269,8 +297,8 @@ void Core::makeTox(QByteArray savedata) case TOX_ERR_NEW_PORT_ALLOC: if (s->getEnableIPv6()) { - tox_options_set_ipv6_enabled(toxOptions.get(), false); - tox = tox_new(toxOptions.get(), &tox_err); + tox_options_set_ipv6_enabled(toxOptions, false); + tox = tox_new(toxOptions, &tox_err); if (tox_err == TOX_ERR_NEW_OK) { qWarning() << "Core failed to start with IPv6, falling back to IPv4. LAN discovery " "may not work properly.";