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).
This commit is contained in:
parent
3b55d780b0
commit
640aaca1ae
2 changed files with 100 additions and 13 deletions
35
cg-net.el
35
cg-net.el
|
|
@ -77,6 +77,20 @@ deliberately."
|
||||||
(string :tag "A specific interface address"))
|
(string :tag "A specific interface address"))
|
||||||
:group 'cg-net)
|
: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
|
(defvar cg-net-state-functions nil
|
||||||
"Abnormal hook run on a client after the game state is updated.
|
"Abnormal hook run on a client after the game state is updated.
|
||||||
Each function is called with the client's game object.")
|
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))))
|
(cg-net--scrub (car (read-from-string line))))
|
||||||
(error (message "cg-net: bad message: %S" err)))))
|
(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
|
;;;; Host
|
||||||
|
|
||||||
|
|
@ -194,7 +215,15 @@ The socket binds `cg-net-host-address' -- by default, this machine only."
|
||||||
(setq cg-net--host nil)))
|
(setq cg-net--host nil)))
|
||||||
|
|
||||||
(defun cg-net--host-accept (_server connection _message)
|
(defun cg-net--host-accept (_server connection _message)
|
||||||
"Set up an accepted CONNECTION: assign a seat and send the current state."
|
"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)))
|
(let ((seat (cg-net-host-next-seat cg-net--host)))
|
||||||
(setf (cg-net-host-next-seat cg-net--host) (1+ seat))
|
(setf (cg-net-host-next-seat cg-net--host) (1+ seat))
|
||||||
(push connection (cg-net-host-clients cg-net--host))
|
(push connection (cg-net-host-clients cg-net--host))
|
||||||
|
|
@ -205,7 +234,7 @@ The socket binds `cg-net-host-address' -- by default, this machine only."
|
||||||
(cg-net--send connection
|
(cg-net--send connection
|
||||||
(list :type 'state
|
(list :type 'state
|
||||||
:state (cg-net-game-state (cg-net-host-game cg-net--host) seat)))
|
: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)))
|
(run-hook-with-args 'cg-net-connect-functions cg-net--host seat))))
|
||||||
|
|
||||||
(defun cg-net--host-handle (proc msg)
|
(defun cg-net--host-handle (proc msg)
|
||||||
"Handle one message MSG from a client PROC on the host."
|
"Handle one message MSG from a client PROC on the host."
|
||||||
|
|
|
||||||
|
|
@ -123,6 +123,64 @@ silent default."
|
||||||
(substring (process-contact srv :local) 0 4)))
|
(substring (process-contact srv :local) 0 4)))
|
||||||
(cg-net-host-stop))))
|
(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
|
;;;; Gaps
|
||||||
|
|
||||||
(ert-deftest cgt-gaps-deal ()
|
(ert-deftest cgt-gaps-deal ()
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue