Enforced absolute maximum read size in GDB server

src/DebugServer/Gdb/Connection.cpp

@@ -175,10 +175,12 @@ namespace Bloom::DebugServer::Gdb
     }
 
     std::vector<unsigned char> Connection::read(
-        size_t bytes,
+        std::optional<std::size_t> bytes,
         bool interruptible,
         std::optional<std::chrono::milliseconds> timeout
     ) {
+        assert(!bytes.has_value() || *bytes <= Connection::ABSOLUTE_MAXIMUM_PACKET_READ_SIZE);
+
         auto output = std::vector<unsigned char>();
         constexpr size_t bufferSize = 1024;
         std::array<unsigned char, bufferSize> 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()) {

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<int> 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<unsigned char> read(
-            std::size_t bytes = 0,
+            std::optional<std::size_t> bytes = std::nullopt,
             bool interruptible = true,
             std::optional<std::chrono::milliseconds> timeout = std::nullopt
         );

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<Events::DebugSessionStarted>());