diff --git a/src/DebugServer/Gdb/Connection.cpp b/src/DebugServer/Gdb/Connection.cpp index 624f94f4..7018a82b 100644 --- a/src/DebugServer/Gdb/Connection.cpp +++ b/src/DebugServer/Gdb/Connection.cpp @@ -175,10 +175,12 @@ namespace Bloom::DebugServer::Gdb } std::vector Connection::read( - size_t bytes, + std::optional bytes, bool interruptible, std::optional timeout ) { + assert(!bytes.has_value() || *bytes <= Connection::ABSOLUTE_MAXIMUM_PACKET_READ_SIZE); + auto output = std::vector(); constexpr size_t bufferSize = 1024; std::array buffer = {}; @@ -209,7 +211,7 @@ namespace Bloom::DebugServer::Gdb throw DebugServerInterrupted(); } - size_t bytesToRead = (bytes > bufferSize || bytes == 0) ? bufferSize : bytes; + size_t bytesToRead = (!bytes.has_value() || bytes > bufferSize) ? bufferSize : *bytes; while ( bytesToRead > 0 && (bytesRead = ::read(this->socketFileDescriptor.value(), buffer.data(), bytesToRead)) > 0 @@ -221,7 +223,19 @@ namespace Bloom::DebugServer::Gdb break; } - bytesToRead = ((bytes - output.size()) > bufferSize || bytes == 0) ? bufferSize : (bytes - output.size()); + if (output.size() >= Connection::ABSOLUTE_MAXIMUM_PACKET_READ_SIZE) { + /* + * We're receiving far too much data from GDB - something is definitely not right here. + * This could be an attempted DoS attack. + * + * Assume the worst and throw an exception. The connection will be killed as a result. + */ + throw ClientCommunicationError("GDB client attempted to send too much data"); + } + + bytesToRead = (!bytes.has_value() || (*bytes - output.size()) > bufferSize) + ? bufferSize + : (*bytes - output.size()); } if (output.empty()) { diff --git a/src/DebugServer/Gdb/Connection.hpp b/src/DebugServer/Gdb/Connection.hpp index 74bb2d43..f5c5ebb5 100644 --- a/src/DebugServer/Gdb/Connection.hpp +++ b/src/DebugServer/Gdb/Connection.hpp @@ -24,6 +24,13 @@ namespace Bloom::DebugServer::Gdb class Connection { public: + /* + * GDB should never attempt to send more than this in a single instance. + * + * In the event that it does, we assume the worst and kill the connection. + */ + static constexpr auto ABSOLUTE_MAXIMUM_PACKET_READ_SIZE = 5120; + explicit Connection(int serverSocketFileDescriptor, EventFdNotifier& interruptEventNotifier); Connection() = delete; @@ -67,15 +74,10 @@ namespace Bloom::DebugServer::Gdb */ void writePacket(const ResponsePackets::ResponsePacket& packet); - [[nodiscard]] int getMaxPacketSize() const { - return this->maxPacketSize; - } - private: std::optional socketFileDescriptor; struct sockaddr_in socketAddress = {}; - int maxPacketSize = 1024; /** * The interruptEventNotifier (instance of EventFdNotifier) allows us to interrupt blocking I/O calls on this @@ -123,7 +125,7 @@ namespace Bloom::DebugServer::Gdb * @return */ std::vector read( - std::size_t bytes = 0, + std::optional bytes = std::nullopt, bool interruptible = true, std::optional timeout = std::nullopt ); diff --git a/src/DebugServer/Gdb/DebugSession.cpp b/src/DebugServer/Gdb/DebugSession.cpp index 45cc12ac..0b550787 100644 --- a/src/DebugServer/Gdb/DebugSession.cpp +++ b/src/DebugServer/Gdb/DebugSession.cpp @@ -1,6 +1,5 @@ #include "DebugSession.hpp" -#include "src/Logger/Logger.hpp" #include "src/EventManager/EventManager.hpp" namespace Bloom::DebugServer::Gdb @@ -15,7 +14,7 @@ namespace Bloom::DebugServer::Gdb , gdbTargetDescriptor(targetDescriptor) { this->supportedFeatures.insert({ - Feature::PACKET_SIZE, std::to_string(this->connection.getMaxPacketSize()) + Feature::PACKET_SIZE, std::to_string(Connection::ABSOLUTE_MAXIMUM_PACKET_READ_SIZE) }); EventManager::triggerEvent(std::make_shared());