From 3b55d780b0e5e60b40bbd9edd5d79d4447a6a1ae Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 20:32:45 -0500 Subject: [PATCH] cg-bid-net: refuse a discard of cards the player does not hold Finding 3 of the 2026-07-29 security review: bids, passes and plays were validated properly, but a kitty discard only checked that five cards were named -- and cg-bid--discard removes cards with cl-set-difference, which silently ignores cards not in the hand. A client naming five cards it did not hold kept all fifteen and played on with more cards than the rules allow. New cg-bid--net-holds-p checks every named card against the seat's hand, respecting multiplicity (five copies of one held card do not pass). The check joins the phase/contractor/arity checks that already live in the discard branch of cg-net-apply-move. Test cgt-bid-net-discard-checked fails against the previous code: the foreign discard was accepted (returned t) and the game advanced to play with a 15-card hand. --- cg-bid-net.el | 14 +++++++++++++- test/card-games-tests.el | 27 +++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/cg-bid-net.el b/cg-bid-net.el index 3ad484c..4f8a145 100644 --- a/cg-bid-net.el +++ b/cg-bid-net.el @@ -143,6 +143,17 @@ and a hand exposed by an open misère is revealed to everyone." ;;;; Apply a move on the host +(defun cg-bid--net-holds-p (hand cards) + "Return non-nil when every card in CARDS is present in HAND. +Multiplicity counts: naming one held card five times is not holding +five cards. Cards are (SUIT . RANK) conses compared with `equal'." + (let ((left (copy-sequence hand))) + (catch 'missing + (dolist (c cards t) + (if (member c left) + (setq left (cl-remove c left :test #'equal :count 1)) + (throw 'missing nil)))))) + (cl-defmethod cg-net-apply-move ((game cg-bid-game) seat move) "Apply MOVE made by absolute SEAT to the host's 500 GAME. MOVE is (bid BID), (pass), (discard CARD...) or (play CARD). Return @@ -157,7 +168,8 @@ non-nil when the move was legal and applied, so the host broadcasts." (cg-bid--auction-act game seat nil) (setq ok t))) (`(discard . ,cards) (when (and (eq phase 'kitty) (eql (cg-get game :contractor) seat) - (= (length cards) 5)) + (= (length cards) 5) + (cg-bid--net-holds-p (cg-bid--hand game seat) cards)) (cg-bid--discard game seat cards) (setq ok t))) (`(play ,card) (when (and (eq phase 'play) (eql (cg-get game :turn) seat) diff --git a/test/card-games-tests.el b/test/card-games-tests.el index f77f312..ccb298d 100644 --- a/test/card-games-tests.el +++ b/test/card-games-tests.el @@ -354,6 +354,33 @@ silent default." (should-not (cg-net-apply-move g 0 '(play (0 . 0)))) ; wrong phase )) +(ert-deftest cgt-bid-net-discard-checked () + "The host rejects a kitty discard of cards the seat does not hold. +Finding 3 of the 2026-07-29 review: the other move checks were real, +but a discard only counted its cards, and cl-set-difference silently +ignores cards that are not in the hand -- so five phantom discards +left a 15-card hand in play." + (let* ((cg-bid--human-seats '(0 1 2 3)) + (g (cg-bid--deal (make-instance 'cg-bid-game) 3))) + ;; put the game where a discard is legal: West won the auction + (cg-put g :phase 'kitty) + (cg-put g :contractor 1) + (cg-bid--set-hand g 1 (append (cg-get g :kitty) (cg-bid--hand g 1))) + (let* ((hand (copy-sequence (cg-bid--hand g 1))) + (foreign (cl-subseq (cg-bid--hand g 2) 0 5))) + ;; five cards West does not hold: refused, nothing moves + (should-not (cg-net-apply-move g 1 (cons 'discard foreign))) + (should (equal hand (cg-bid--hand g 1))) + (should (eq 'kitty (cg-get g :phase))) + ;; one held card named five times: also refused + (should-not (cg-net-apply-move g 1 + (cons 'discard (make-list 5 (car hand))))) + (should (eq 'kitty (cg-get g :phase))) + ;; an honest five from the hand is accepted and play begins + (should (cg-net-apply-move g 1 (cons 'discard (cl-subseq hand 0 5)))) + (should (eq 'play (cg-get g :phase))) + (should (= 10 (length (cg-bid--hand g 1))))))) + (ert-deftest cgt-bid-net-loopback () "A 500 move travels client -> host -> filtered broadcast over TCP." (condition-case _