From 327358a589438ba3e729a61ee20c13c01188adae Mon Sep 17 00:00:00 2001 From: Matt Whitlock Date: Fri, 5 Nov 2021 16:36:32 -0400 Subject: [PATCH] mitigate potential DoS vector by limiting pending piece uploads Previously there was no explicit limit on the number of piece requests a peer could have outstanding. Each such request can potentially consume memory up to the piece size of the torrent. Worse, since there is no check for redundant requests, it was trivial for a peer to cause us to consume unbounded amounts of memory by sending a long run of requests for oversized pieces. (N.B.: Other implementations disconnect peers for requesting more than 128 KiB at a time, but KTorrent allows any requested size up to the piece size.) This commit implements limits on the number of piece block uploads a peer may have pending at any one time and on the total byte count of those uploads. The limits are 512 uploads and 8 MiB of piece data. Any requests received in excess of those limits are rejected. Note that popular BitTorrent client implementations do not queue more than a handful of piece requests at any one time, so these new limits are expected to be proactively defensive only. That said, the author has repeatedly observed KTorrent consuming over half a gigabyte of RAM in queued PIECE messages waiting to be transmitted. The new limits are intended to alleviate this pain. --- src/net/packetsocket.cpp | 19 ++++++++++++++++--- src/net/packetsocket.h | 4 ++++ src/peer/peer.cpp | 8 ++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/net/packetsocket.cpp b/src/net/packetsocket.cpp index 7095b28..80c7a67 100644 --- a/src/net/packetsocket.cpp +++ b/src/net/packetsocket.cpp @@ -15,6 +15,7 @@ namespace net PacketSocket::PacketSocket(SocketDevice *sock) : TrafficShapedSocket(sock) , ctrl_packets_sent(0) + , pending_upload_data_bytes(0) , uploaded_data_bytes(0) { } @@ -22,6 +23,7 @@ PacketSocket::PacketSocket(SocketDevice *sock) PacketSocket::PacketSocket(int fd, int ip_version) : TrafficShapedSocket(fd, ip_version) , ctrl_packets_sent(0) + , pending_upload_data_bytes(0) , uploaded_data_bytes(0) { } @@ -29,6 +31,7 @@ PacketSocket::PacketSocket(int fd, int ip_version) PacketSocket::PacketSocket(bool tcp, int ip_version) : TrafficShapedSocket(tcp, ip_version) , ctrl_packets_sent(0) + , pending_upload_data_bytes(0) , uploaded_data_bytes(0) { } @@ -82,6 +85,7 @@ Uint32 PacketSocket::write(Uint32 max, bt::TimeStamp now) QMutexLocker locker(&mutex); if (curr_packet->getType() == PIECE) { up_speed->onData(ret, now); + pending_upload_data_bytes -= ret; uploaded_data_bytes += ret; } } else @@ -113,10 +117,12 @@ Uint32 PacketSocket::write(Uint32 max, bt::TimeStamp now) void PacketSocket::addPacket(Packet::Ptr packet) { + Q_ASSERT(!packet->sending()); QMutexLocker locker(&mutex); - if (packet->getType() == PIECE) + if (packet->getType() == PIECE) { data_packets.push_back(packet); - else + pending_upload_data_bytes += packet->getDataLength(); + } else control_packets.push_back(packet); // tell upload thread we have data ready should it be sleeping net::SocketMonitor::instance().signalPacketReady(); @@ -151,7 +157,7 @@ void PacketSocket::clearPieces(bool reject) if (p->getType() == bt::PIECE && !p->sending() && curr_packet != p) { if (reject) addPacket(Packet::Ptr(p->makeRejectOfPiece())); - + pending_upload_data_bytes -= p->getDataLength(); i = data_packets.erase(i); } else { i++; @@ -166,6 +172,7 @@ void PacketSocket::doNotSendPiece(const bt::Request &req, bool reject) while (i != data_packets.end()) { Packet::Ptr p = *i; if (p->isPiece(req) && !p->sending() && p != curr_packet) { + pending_upload_data_bytes -= p->getDataLength(); i = data_packets.erase(i); if (reject) { // queue a reject packet @@ -183,4 +190,10 @@ Uint32 PacketSocket::numPendingPieceUploads() const return data_packets.size(); } +Uint32 PacketSocket::numPendingPieceUploadBytes() const +{ + QMutexLocker locker(&mutex); + return pending_upload_data_bytes; +} + } diff --git a/src/net/packetsocket.h b/src/net/packetsocket.h index bbcb9e5..8d06bbc 100644 --- a/src/net/packetsocket.h +++ b/src/net/packetsocket.h @@ -61,6 +61,9 @@ public: /// Get the number of pending piece uploads Uint32 numPendingPieceUploads() const; + /// Get the number of pending piece upload bytes (including message headers) + Uint32 numPendingPieceUploadBytes() const; + protected: /** * Preprocess the packet data, before it is sent. Default implementation does nothing. @@ -77,6 +80,7 @@ protected: bt::Packet::Ptr curr_packet; Uint32 ctrl_packets_sent; + Uint32 pending_upload_data_bytes; Uint32 uploaded_data_bytes; }; diff --git a/src/peer/peer.cpp b/src/peer/peer.cpp index 0f0fffd..09a7b9d 100644 --- a/src/peer/peer.cpp +++ b/src/peer/peer.cpp @@ -32,6 +32,8 @@ using namespace net; namespace bt { static const int MAX_METADATA_SIZE = 100 * 1024 * 1024; // maximum allowed metadata_size (up to 100 MiB) +static const unsigned MAX_PENDING_UPLOAD_BLOCKS = 512; // allow up to 8 MiB of 16KiB blocks +static const Uint32 MAX_PENDING_UPLOAD_BYTES = MAX_PENDING_UPLOAD_BLOCKS * (13 + 16384); static Uint32 peer_id_counter = 1; bool Peer::resolve_hostname = true; @@ -818,6 +820,12 @@ bool Peer::sendChunk(Uint32 index, Uint32 begin, Uint32 len, Chunk *ch) Out(SYS_CON | LOG_NOTICE) << "\tPiece : begin = " << begin << " len = " << len << endl; return false; } + if (sock->numPendingPieceUploads() >= MAX_PENDING_UPLOAD_BLOCKS || + sock->numPendingPieceUploadBytes() + 13 + len > MAX_PENDING_UPLOAD_BYTES) + { + Out(SYS_CON | LOG_NOTICE) << "Warning : rejecting piece request due to limit on pending uploads" << endl; + return false; + } /* Out(SYS_CON|LOG_DEBUG) << QString("Uploading %1 %2 %3 %4 %5") * .arg(index).arg(begin).arg(len).arg((quint64)ch,0,16).arg((quint64)ch->getData(),0,16) * << endl;; -- GitLab