From 640aaca1ae0450adef096ccf265d0e3178066861 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 20:34:19 -0500 Subject: [PATCH] cg-net: bound the line buffer and the connection count Finding 4 of the 2026-07-29 security review: incoming data was collected until a newline arrived with no limit on how much, so a connection sending forever without a newline grew the buffer until memory ran out; the client list could grow the same way. No login was needed (Finding 1). Two bounds, both defcustoms: cg-net-max-line (64 KiB -- generous for a card game) closes any connection whose pending newline-less data exceeds it, and cg-net-max-connections (8) closes connections arriving past the limit before they are seated. Finding 1's loopback default narrows who can reach the port; this bounds what anyone who does reach it can consume -- a mitigation that depends on another setting staying at its default is not a bound. Tests cgt-net-line-cap and cgt-net-connection-cap fail against the previous code: the 80 KiB flooder stayed connected, and a third connection stayed live past a cap of two (= 2 3). --- cg-net.el | 55 ++++++++++++++++++++++++++++--------- test/card-games-tests.el | 58 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 13 deletions(-) diff --git a/cg-net.el b/cg-net.el index 4fcf79d..cb1666e 100644 --- a/cg-net.el +++ b/cg-net.el @@ -77,6 +77,20 @@ deliberately." (string :tag "A specific interface address")) :group 'cg-net) +(defcustom cg-net-max-line 65536 + "Longest unterminated line accepted from a connection, in bytes. +Messages in this protocol are short; 64 KiB is generous. A connection +whose pending (newline-less) data exceeds this is closed, so one peer +cannot grow the line buffer until memory runs out." + :type 'integer :group 'cg-net) + +(defcustom cg-net-max-connections 8 + "Most simultaneous client connections a host will accept. +A table seats four, so the default leaves headroom without letting the +client list grow unboundedly. Connections beyond the limit are closed +as they arrive." + :type 'integer :group 'cg-net) + (defvar cg-net-state-functions nil "Abnormal hook run on a client after the game state is updated. Each function is called with the client's game object.") @@ -159,7 +173,14 @@ properties stripped (`cg-net--scrub') before HANDLER sees them." (cg-net--scrub (car (read-from-string line)))) (error (message "cg-net: bad message: %S" err))))) ) - (process-put proc 'cg-net-buf (substring buf start))))) + (let ((rest (substring buf start))) + (if (> (length rest) cg-net-max-line) + (progn + (process-put proc 'cg-net-buf nil) + (message "cg-net: dropping %s (line over %d bytes)" + (process-name proc) cg-net-max-line) + (delete-process proc)) + (process-put proc 'cg-net-buf rest)))))) ;;;; Host @@ -194,18 +215,26 @@ The socket binds `cg-net-host-address' -- by default, this machine only." (setq cg-net--host nil))) (defun cg-net--host-accept (_server connection _message) - "Set up an accepted CONNECTION: assign a seat and send the current state." - (let ((seat (cg-net-host-next-seat cg-net--host))) - (setf (cg-net-host-next-seat cg-net--host) (1+ seat)) - (push connection (cg-net-host-clients cg-net--host)) - (process-put connection 'cg-net-seat seat) - (set-process-coding-system connection 'utf-8 'utf-8) - (set-process-filter connection (cg-net--filter #'cg-net--host-handle)) - (cg-net--send connection (list :type 'welcome :seat seat)) - (cg-net--send connection - (list :type 'state - :state (cg-net-game-state (cg-net-host-game cg-net--host) seat))) - (run-hook-with-args 'cg-net-connect-functions cg-net--host seat))) + "Set up an accepted CONNECTION: assign a seat and send the current state. +A connection arriving past `cg-net-max-connections' is closed instead." + (if (>= (length (cl-remove-if-not #'process-live-p + (cg-net-host-clients cg-net--host))) + cg-net-max-connections) + (progn + (message "cg-net: refusing connection (table is at %d)" + cg-net-max-connections) + (delete-process connection)) + (let ((seat (cg-net-host-next-seat cg-net--host))) + (setf (cg-net-host-next-seat cg-net--host) (1+ seat)) + (push connection (cg-net-host-clients cg-net--host)) + (process-put connection 'cg-net-seat seat) + (set-process-coding-system connection 'utf-8 'utf-8) + (set-process-filter connection (cg-net--filter #'cg-net--host-handle)) + (cg-net--send connection (list :type 'welcome :seat seat)) + (cg-net--send connection + (list :type 'state + :state (cg-net-game-state (cg-net-host-game cg-net--host) seat))) + (run-hook-with-args 'cg-net-connect-functions cg-net--host seat)))) (defun cg-net--host-handle (proc msg) "Handle one message MSG from a client PROC on the host." diff --git a/test/card-games-tests.el b/test/card-games-tests.el index ccb298d..cc0f046 100644 --- a/test/card-games-tests.el +++ b/test/card-games-tests.el @@ -123,6 +123,64 @@ silent default." (substring (process-contact srv :local) 0 4))) (cg-net-host-stop)))) +(ert-deftest cgt-net-line-cap () + "A connection sending endless bytes with no newline is dropped. +Finding 4 of the 2026-07-29 review: the partial-line buffer grew +without limit, so one connection could consume all available memory. +It is now bounded by `cg-net-max-line'." + (condition-case _ + (delete-process + (make-network-process :name "cgt-probe5" :server t :service 0 + :host "127.0.0.1" :family 'ipv4)) + (error (ert-skip "TCP not available"))) + (cl-flet ((pump () (dotimes (_ 8) (accept-process-output nil 0.05)))) + (let* ((hgame (make-instance 'cgt-net-game :env (list :counter 0))) + (srv (cg-net-host-start hgame 0)) + (port (process-contact srv :service)) + (raw (make-network-process :name "cgt-flood" :host "127.0.0.1" + :service port :family 'ipv4))) + (unwind-protect + (progn + (pump) + (let ((chunk (make-string 8192 ?a))) + (cl-loop repeat 10 while (process-live-p raw) + do (ignore-errors (process-send-string raw chunk)) + (pump))) + ;; 80 KiB with no newline: the host must have hung up on us + (should-not (process-live-p raw))) + (when (process-live-p raw) (delete-process raw)) + (cg-net-host-stop))))) + +(ert-deftest cgt-net-connection-cap () + "Connections beyond `cg-net-max-connections' are refused. +Finding 4's second bound: the client list cannot grow without limit." + (condition-case _ + (delete-process + (make-network-process :name "cgt-probe6" :server t :service 0 + :host "127.0.0.1" :family 'ipv4)) + (error (ert-skip "TCP not available"))) + (cl-flet ((pump () (dotimes (_ 8) (accept-process-output nil 0.05)))) + (let* ((cg-net-max-connections 2) + (hgame (make-instance 'cgt-net-game :env (list :counter 0))) + (srv (cg-net-host-start hgame 0)) + (port (process-contact srv :service)) + (procs nil)) + (unwind-protect + (progn + (dotimes (i 3) + (push (make-network-process :name (format "cgt-c%d" i) + :host "127.0.0.1" :service port + :family 'ipv4) + procs) + (pump)) + (should (= 2 (length (cl-remove-if-not + #'process-live-p + (cg-net-host-clients cg-net--host))))) + ;; the newest connection is the one turned away + (should-not (process-live-p (car procs)))) + (dolist (p procs) (when (process-live-p p) (delete-process p))) + (cg-net-host-stop))))) + ;;;; Gaps (ert-deftest cgt-gaps-deal ()