From 80a5a8ea2b7ad512c74c29df5b504e9be6cf23a0 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Wed, 10 Mar 2021 12:07:08 +0100 Subject: i2p: limit the size of incoming messages Put a limit on the amount of data `Sock::RecvUntilTerminator()` can read if no terminator is received. In the case of I2P this avoids a runaway (or malicious) I2P proxy sending us tons of data without a terminator before a timeout is triggered. --- src/i2p.cpp | 4 ++-- src/i2p.h | 8 ++++++++ src/util/sock.cpp | 10 ++++++++-- src/util/sock.h | 5 ++++- 4 files changed, 22 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/i2p.cpp b/src/i2p.cpp index 42270deaeb..d16c620d88 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -153,7 +153,7 @@ bool Session::Accept(Connection& conn) } const std::string& peer_dest = - conn.sock.RecvUntilTerminator('\n', MAX_WAIT_FOR_IO, *m_interrupt); + conn.sock.RecvUntilTerminator('\n', MAX_WAIT_FOR_IO, *m_interrupt, MAX_MSG_SIZE); conn.peer = CService(DestB64ToAddr(peer_dest), Params().GetDefaultPort()); @@ -252,7 +252,7 @@ Session::Reply Session::SendRequestAndGetReply(const Sock& sock, // signaled. static constexpr auto recv_timeout = 3min; - reply.full = sock.RecvUntilTerminator('\n', recv_timeout, *m_interrupt); + reply.full = sock.RecvUntilTerminator('\n', recv_timeout, *m_interrupt, MAX_MSG_SIZE); for (const auto& kv : spanparsing::Split(reply.full, ' ')) { const auto& pos = std::find(kv.begin(), kv.end(), '='); diff --git a/src/i2p.h b/src/i2p.h index 8fafe0a4d0..1ebe7d0329 100644 --- a/src/i2p.h +++ b/src/i2p.h @@ -40,6 +40,14 @@ struct Connection { namespace sam { +/** + * The maximum size of an incoming message from the I2P SAM proxy (in bytes). + * Used to avoid a runaway proxy from sending us an "unlimited" amount of data without a terminator. + * The longest known message is ~1400 bytes, so this is high enough not to be triggered during + * normal operation, yet low enough to avoid a malicious proxy from filling our memory. + */ +static constexpr size_t MAX_MSG_SIZE{65536}; + /** * I2P SAM session. */ diff --git a/src/util/sock.cpp b/src/util/sock.cpp index e13c52a16a..f9ecfef5d4 100644 --- a/src/util/sock.cpp +++ b/src/util/sock.cpp @@ -175,7 +175,8 @@ void Sock::SendComplete(const std::string& data, std::string Sock::RecvUntilTerminator(uint8_t terminator, std::chrono::milliseconds timeout, - CThreadInterrupt& interrupt) const + CThreadInterrupt& interrupt, + size_t max_data) const { const auto deadline = GetTime() + timeout; std::string data; @@ -190,9 +191,14 @@ std::string Sock::RecvUntilTerminator(uint8_t terminator, // at a time is about 50 times slower. for (;;) { + if (data.size() >= max_data) { + throw std::runtime_error( + strprintf("Received too many bytes without a terminator (%u)", data.size())); + } + char buf[512]; - const ssize_t peek_ret{Recv(buf, sizeof(buf), MSG_PEEK)}; + const ssize_t peek_ret{Recv(buf, std::min(sizeof(buf), max_data - data.size()), MSG_PEEK)}; switch (peek_ret) { case -1: { diff --git a/src/util/sock.h b/src/util/sock.h index ecebb84205..4b0618dcff 100644 --- a/src/util/sock.h +++ b/src/util/sock.h @@ -135,13 +135,16 @@ public: * @param[in] terminator Character up to which to read from the socket. * @param[in] timeout Timeout for the entire operation. * @param[in] interrupt If this is signaled then the operation is canceled. + * @param[in] max_data The maximum amount of data (in bytes) to receive. If this many bytes + * are received and there is still no terminator, then this method will throw an exception. * @return The data that has been read, without the terminating character. * @throws std::runtime_error if the operation cannot be completed. In this case some bytes may * have been consumed from the socket. */ virtual std::string RecvUntilTerminator(uint8_t terminator, std::chrono::milliseconds timeout, - CThreadInterrupt& interrupt) const; + CThreadInterrupt& interrupt, + size_t max_data) const; /** * Check if still connected. -- cgit v1.2.3