From 431658bee69f20eefa98dcacfe4cc23ffb2ca053 Mon Sep 17 00:00:00 2001 From: sledgehammer999 Date: Tue, 7 Mar 2017 01:34:55 +0200 Subject: [PATCH] Fix race condition where there was a chance to allow all ips between reparsings of the ip filter. --- .../bittorrent/private/filterparserthread.cpp | 42 +++++++++---------- .../bittorrent/private/filterparserthread.h | 21 ++++------ src/base/bittorrent/session.cpp | 42 ++++++++++++------- src/base/bittorrent/session.h | 3 +- 4 files changed, 59 insertions(+), 49 deletions(-) diff --git a/src/base/bittorrent/private/filterparserthread.cpp b/src/base/bittorrent/private/filterparserthread.cpp index 250893e0f..8c06f5e1b 100644 --- a/src/base/bittorrent/private/filterparserthread.cpp +++ b/src/base/bittorrent/private/filterparserthread.cpp @@ -33,17 +33,13 @@ #include #include -#include -#include - #include "base/logger.h" #include "filterparserthread.h" namespace libt = libtorrent; -FilterParserThread::FilterParserThread(libt::session *s, QObject *parent) +FilterParserThread::FilterParserThread(QObject *parent) : QThread(parent) - , m_session(s) , m_abort(false) { } @@ -55,10 +51,10 @@ FilterParserThread::~FilterParserThread() } // Parser for eMule ip filter in DAT format -int FilterParserThread::parseDATFilterFile(QString filePath, libt::ip_filter &filter) +int FilterParserThread::parseDATFilterFile() { int ruleCount = 0; - QFile file(filePath); + QFile file(m_filePath); if (!file.exists()) return ruleCount; if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) { @@ -136,7 +132,7 @@ int FilterParserThread::parseDATFilterFile(QString filePath, libt::ip_filter &fi // Now Add to the filter try { - filter.add_rule(startAddr, endAddr, libt::ip_filter::blocked); + m_filter.add_rule(startAddr, endAddr, libt::ip_filter::blocked); ++ruleCount; } catch(std::exception &) { @@ -149,10 +145,10 @@ int FilterParserThread::parseDATFilterFile(QString filePath, libt::ip_filter &fi } // Parser for PeerGuardian ip filter in p2p format -int FilterParserThread::parseP2PFilterFile(QString filePath, libt::ip_filter &filter) +int FilterParserThread::parseP2PFilterFile() { int ruleCount = 0; - QFile file(filePath); + QFile file(m_filePath); if (!file.exists()) return ruleCount; if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) { @@ -219,7 +215,7 @@ int FilterParserThread::parseP2PFilterFile(QString filePath, libt::ip_filter &fi } try { - filter.add_rule(startAddr, endAddr, libt::ip_filter::blocked); + m_filter.add_rule(startAddr, endAddr, libt::ip_filter::blocked); ++ruleCount; } catch(std::exception &) { @@ -257,10 +253,10 @@ int FilterParserThread::getlineInStream(QDataStream &stream, std::string &name, } // Parser for PeerGuardian ip filter in p2p format -int FilterParserThread::parseP2BFilterFile(QString filePath, libt::ip_filter &filter) +int FilterParserThread::parseP2BFilterFile() { int ruleCount = 0; - QFile file(filePath); + QFile file(m_filePath); if (!file.exists()) return ruleCount; if (!file.open(QIODevice::ReadOnly)) { @@ -298,7 +294,7 @@ int FilterParserThread::parseP2BFilterFile(QString filePath, libt::ip_filter &fi libt::address_v4 last(ntohl(end)); // Apply to bittorrent session try { - filter.add_rule(first, last, libt::ip_filter::blocked); + m_filter.add_rule(first, last, libt::ip_filter::blocked); ++ruleCount; } catch(std::exception &) {} @@ -348,7 +344,7 @@ int FilterParserThread::parseP2BFilterFile(QString filePath, libt::ip_filter &fi libt::address_v4 last(ntohl(end)); // Apply to bittorrent session try { - filter.add_rule(first, last, libt::ip_filter::blocked); + m_filter.add_rule(first, last, libt::ip_filter::blocked); ++ruleCount; } catch(std::exception &) {} @@ -369,7 +365,7 @@ int FilterParserThread::parseP2BFilterFile(QString filePath, libt::ip_filter &fi // * eMule IP list (DAT): http://wiki.phoenixlabs.org/wiki/DAT_Format // * PeerGuardian Text (P2P): http://wiki.phoenixlabs.org/wiki/P2P_Format // * PeerGuardian Binary (P2B): http://wiki.phoenixlabs.org/wiki/P2B_Format -void FilterParserThread::processFilterFile(QString filePath) +void FilterParserThread::processFilterFile(const QString &filePath) { if (isRunning()) { // Already parsing a filter, m_abort first @@ -379,10 +375,16 @@ void FilterParserThread::processFilterFile(QString filePath) m_abort = false; m_filePath = filePath; + m_filter = libt::ip_filter(); // Run it start(); } +libt::ip_filter FilterParserThread::IPfilter() +{ + return m_filter; +} + QString FilterParserThread::cleanupIPAddress(QString _ip) { _ip = _ip.trimmed(); @@ -417,25 +419,23 @@ QString FilterParserThread::cleanupIPAddress(QString _ip) void FilterParserThread::run() { qDebug("Processing filter file"); - libt::ip_filter filter = m_session->get_ip_filter(); int ruleCount = 0; if (m_filePath.endsWith(".p2p", Qt::CaseInsensitive)) { // PeerGuardian p2p file - ruleCount = parseP2PFilterFile(m_filePath, filter); + ruleCount = parseP2PFilterFile(); } else if (m_filePath.endsWith(".p2b", Qt::CaseInsensitive)) { // PeerGuardian p2b file - ruleCount = parseP2BFilterFile(m_filePath, filter); + ruleCount = parseP2BFilterFile(); } else if (m_filePath.endsWith(".dat", Qt::CaseInsensitive)) { // eMule DAT format - ruleCount = parseDATFilterFile(m_filePath, filter); + ruleCount = parseDATFilterFile(); } if (m_abort) return; try { - m_session->set_ip_filter(filter); emit IPFilterParsed(ruleCount); } catch(std::exception &) { diff --git a/src/base/bittorrent/private/filterparserthread.h b/src/base/bittorrent/private/filterparserthread.h index eb06e6706..5b7fe1cca 100644 --- a/src/base/bittorrent/private/filterparserthread.h +++ b/src/base/bittorrent/private/filterparserthread.h @@ -33,23 +33,20 @@ #include +#include + class QDataStream; class QStringList; -namespace libtorrent -{ - class session; - struct ip_filter; -} - class FilterParserThread : public QThread { Q_OBJECT public: - FilterParserThread(libtorrent::session *s, QObject *parent = 0); + FilterParserThread(QObject *parent = 0); ~FilterParserThread(); - void processFilterFile(QString filePath); + void processFilterFile(const QString &filePath); + libtorrent::ip_filter IPfilter(); signals: void IPFilterParsed(int ruleCount); @@ -60,14 +57,14 @@ protected: void run(); private: - int parseDATFilterFile(QString filePath, libtorrent::ip_filter &filter); - int parseP2PFilterFile(QString filePath, libtorrent::ip_filter &filter); + int parseDATFilterFile(); + int parseP2PFilterFile(); int getlineInStream(QDataStream &stream, std::string &name, char delim); - int parseP2BFilterFile(QString filePath, libtorrent::ip_filter &filter); + int parseP2BFilterFile(); - libtorrent::session *m_session; bool m_abort; QString m_filePath; + libtorrent::ip_filter m_filter; }; #endif // BITTORRENT_FILTERPARSERTHREAD_H diff --git a/src/base/bittorrent/session.cpp b/src/base/bittorrent/session.cpp index 5669ba629..dca609e1a 100644 --- a/src/base/bittorrent/session.cpp +++ b/src/base/bittorrent/session.cpp @@ -385,10 +385,16 @@ Session::Session(QObject *parent) .arg(encryption() == 0 ? tr("ON") : encryption() == 1 ? tr("FORCED") : tr("OFF")) , Log::INFO); - if (isIPFilteringEnabled()) + if (isIPFilteringEnabled()) { + // Manually banned IPs are handled in that function too(in the slots) enableIPFilter(); - // Add the banned IPs - processBannedIPs(); + } + else { + // Add the banned IPs + libt::ip_filter filter; + processBannedIPs(filter); + m_nativeSession->set_ip_filter(filter); + } m_categories = map_cast(m_storedCategories); if (isSubcategoriesEnabled()) { @@ -891,10 +897,9 @@ void Session::configure() qDebug("Session configured"); } -void Session::processBannedIPs() +void Session::processBannedIPs(libt::ip_filter &filter) { // First, import current filter - libt::ip_filter filter = m_nativeSession->get_ip_filter(); foreach (const QString &ip, m_bannedIPs.value()) { boost::system::error_code ec; libt::address addr = libt::address::from_string(ip.toLatin1().constData(), ec); @@ -902,8 +907,6 @@ void Session::processBannedIPs() if (!ec) filter.add_rule(addr, addr, libt::ip_filter::blocked); } - - m_nativeSession->set_ip_filter(filter); } #if LIBTORRENT_VERSION_NUM >= 10100 @@ -3028,13 +3031,12 @@ void Session::configureDeferred() void Session::enableIPFilter() { qDebug("Enabling IPFilter"); - // 1. Clear existing ban list - // 2. Add manual ban list - // 3. Add 3rd party ban list - m_nativeSession->set_ip_filter(libt::ip_filter()); - processBannedIPs(); + // 1. Parse the IP filter + // 2. In the slot add the manually banned IPs to the provided libtorrent::ip_filter + // 3. Set the ip_filter in one go so there isn't a time window where there isn't an ip_filter + // set between clearing the old one and setting the new one. if (!m_filterParser) { - m_filterParser = new FilterParserThread(m_nativeSession, this); + m_filterParser = new FilterParserThread(this); connect(m_filterParser.data(), SIGNAL(IPFilterParsed(int)), SLOT(handleIPFilterParsed(int))); connect(m_filterParser.data(), SIGNAL(IPFilterError()), SLOT(handleIPFilterError())); } @@ -3045,7 +3047,6 @@ void Session::enableIPFilter() void Session::disableIPFilter() { qDebug("Disabling IPFilter"); - m_nativeSession->set_ip_filter(libt::ip_filter()); if (m_filterParser) { disconnect(m_filterParser.data(), 0, this, 0); delete m_filterParser; @@ -3054,7 +3055,9 @@ void Session::disableIPFilter() // Add the banned IPs after the IPFilter disabling // which creates an empty filter and overrides all previously // applied bans. - processBannedIPs(); + libt::ip_filter filter; + processBannedIPs(filter); + m_nativeSession->set_ip_filter(filter); } void Session::recursiveTorrentDownload(const InfoHash &hash) @@ -3191,12 +3194,21 @@ void Session::refresh() void Session::handleIPFilterParsed(int ruleCount) { + if (!m_filterParser) { + libt::ip_filter filter = m_filterParser->IPfilter(); + processBannedIPs(filter); + m_nativeSession->set_ip_filter(filter); + } Logger::instance()->addMessage(tr("Successfully parsed the provided IP filter: %1 rules were applied.", "%1 is a number").arg(ruleCount)); emit IPFilterParsed(false, ruleCount); } void Session::handleIPFilterError() { + libt::ip_filter filter; + processBannedIPs(filter); + m_nativeSession->set_ip_filter(filter); + Logger::instance()->addMessage(tr("Error: Failed to parse the provided IP filter."), Log::CRITICAL); emit IPFilterParsed(true, 0); } diff --git a/src/base/bittorrent/session.h b/src/base/bittorrent/session.h index f5be62299..adeae914a 100644 --- a/src/base/bittorrent/session.h +++ b/src/base/bittorrent/session.h @@ -57,6 +57,7 @@ namespace libtorrent struct torrent_handle; class entry; struct add_torrent_params; + struct ip_filter; struct pe_settings; #if LIBTORRENT_VERSION_NUM < 10100 struct session_settings; @@ -449,7 +450,7 @@ namespace BitTorrent void adjustLimits(libtorrent::settings_pack &settingsPack); #endif void adjustLimits(); - void processBannedIPs(); + void processBannedIPs(libtorrent::ip_filter &filter); const QStringList getListeningIPs(); void configureListeningInterface(); void changeSpeedLimitMode_impl(bool alternative);