fix(server): validate packet length header before parsing

boswatch/network/server.py

@@ -50,7 +50,12 @@ def handle(self):
                 if not len(header):
                     break  # empty data -> socked closed
 
-                length = int(header.strip())
+                try:
+                    length = int(header.strip())
+                except ValueError:
+                    logging.error("%s sent an invalid packet header (expected an integer length, got %r) - closing connection", req_name, header)
+                    break
+
                 data = self.request.recv(length).decode("utf-8")
 
                 if data == "<keep-alive>":

test/boswatch/test_ServerClient.py

@@ -17,6 +17,7 @@
 # problem of the pytest fixtures
 # pylint: disable=redefined-outer-name
 import logging
+import socket
 import time
 import queue
 import pytest
@@ -119,6 +120,30 @@ def test_clientCommunicate(getClient, getRunningServer):
     assert getClient.disconnect()
 
 
+def test_serverInvalidHeaderClosesConnectionOnly(getRunningServer, caplog):
+    r"""!Send a non-integer length header - server must log a clear error and
+    drop just that connection, without crashing or affecting other clients"""
+    rawSock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    rawSock.settimeout(3)
+    rawSock.connect(("localhost", 8080))
+    with caplog.at_level(logging.ERROR):
+        rawSock.sendall(b"bad_head!!")  # 10 bytes (HEADERSIZE), not a valid int
+        time.sleep(0.2)  # let the server thread process and close
+
+    assert any("invalid packet header" in record.message for record in caplog.records)
+
+    # the malformed connection must be closed, not left hanging
+    assert rawSock.recv(1) == b""
+    rawSock.close()
+
+    # the server itself (and other clients) must be unaffected
+    normalClient = TCPClient()
+    assert normalClient.connect()
+    assert normalClient.transmit("test")
+    assert normalClient.receive() == "[ack]"
+    assert normalClient.disconnect()
+
+
 @pytest.mark.skip("needs fixture for more than one client")
 def test_clientMultiCommunicate(getServer):
     r"""!Try to send data to the server with 3 clients and check on '[ack]'"""