From 8f06a43d93c22d766e11b649c178b7abc198b877 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Tue, 22 Sep 2026 21:03:30 +0200 Subject: [PATCH] Fix gosec bounds in the byte swizzle, drop the now-unused releaseCapture, and make the SSH and approval-reuse tests actually exercise their paths --- client/server/capture.go | 10 ---------- .../frontend/src/modules/approval/ApprovalDialog.tsx | 12 ++++++++++-- client/vnc/server/swizzle.go | 9 +++++---- .../types/policy_authorized_users_security_test.go | 2 ++ 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/client/server/capture.go b/client/server/capture.go index e57cd0921..778f32d1f 100644 --- a/client/server/capture.go +++ b/client/server/capture.go @@ -370,16 +370,6 @@ func (s *Server) evictActiveCaptureLocked() func() { } } -// releaseCapture clears the active-capture owner if it still matches sess. -func (s *Server) releaseCapture(sess *capture.Session) { - s.mutex.Lock() - defer s.mutex.Unlock() - if s.activeCapture == sess { - s.activeCapture = nil - s.activeCaptureCancel = nil - } -} - // clearCaptureIfOwner clears engine's capture slot only if sess still owns it. func (s *Server) clearCaptureIfOwner(sess *capture.Session, engine *internal.Engine) { s.mutex.Lock() diff --git a/client/ui/frontend/src/modules/approval/ApprovalDialog.tsx b/client/ui/frontend/src/modules/approval/ApprovalDialog.tsx index eca4409f6..67c6324c8 100644 --- a/client/ui/frontend/src/modules/approval/ApprovalDialog.tsx +++ b/client/ui/frontend/src/modules/approval/ApprovalDialog.tsx @@ -43,10 +43,16 @@ export default function ApprovalDialog() { const peerPubKey = params.get("peer_pubkey") ?? ""; const expiresAt = params.get("expires_at") ?? ""; + // requestID is a dependency because the window is reused across prompts. A + // request that carries no usable expires_at falls back to a deadline + // measured from now, and without recomputing it here the next prompt + // inherits the previous request's deadline and can close the moment it + // opens. const deadline = useMemo(() => { const parsed = Date.parse(expiresAt); return Number.isFinite(parsed) ? parsed : Date.now() + FALLBACK_SECONDS * 1000; - }, [expiresAt]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [expiresAt, requestID]); const title = useMemo(() => { switch (kind) { @@ -125,9 +131,11 @@ export default function ApprovalDialog() { setBusy(false); setArmed(false); closedRef.current = false; + setRemaining(secondsLeft()); const id = globalThis.setTimeout(() => setArmed(true), ARMING_MS); return () => globalThis.clearTimeout(id); - }, [requestID]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [requestID, deadline]); useEffect(() => { const id = globalThis.setInterval(() => { const left = secondsLeft(); diff --git a/client/vnc/server/swizzle.go b/client/vnc/server/swizzle.go index 98c988a04..1930831c1 100644 --- a/client/vnc/server/swizzle.go +++ b/client/vnc/server/swizzle.go @@ -44,10 +44,11 @@ func swizzleBGRAtoRGBA(dst, src []byte) { } // swizzleBGRAtoRGBABytes is the byte-order-independent form, used on big-endian -// targets. dst and src must be the same length and a multiple of 4. +// targets. It converts whole pixels for as long as both sides have one left. func swizzleBGRAtoRGBABytes(dst, src []byte) { - for i := 0; i < len(src); i += 4 { - b, g, r := src[i], src[i+1], src[i+2] - dst[i], dst[i+1], dst[i+2], dst[i+3] = r, g, b, 0xFF + for i := 0; i+4 <= len(src) && i+4 <= len(dst); i += 4 { + s := src[i : i+4 : i+4] + d := dst[i : i+4 : i+4] + d[0], d[1], d[2], d[3] = s[2], s[1], s[0], 0xFF } } diff --git a/shared/management/types/policy_authorized_users_security_test.go b/shared/management/types/policy_authorized_users_security_test.go index 166567547..b837acd06 100644 --- a/shared/management/types/policy_authorized_users_security_test.go +++ b/shared/management/types/policy_authorized_users_security_test.go @@ -101,6 +101,7 @@ func TestApplyResolvedRule_SSHSkipsSourcePeer(t *testing.T) { } rule := &nmdata.PolicyRule{ Protocol: string(PolicyRuleProtocolNetbirdSSH), + Action: string(PolicyTrafficActionAccept), Bidirectional: bidirectional, } state := NewPeerConnResolveState() @@ -153,6 +154,7 @@ func TestApplyResolvedRule_SSHEnablesDestinationPeer(t *testing.T) { func TestApplyResolvedRule_LegacySSHSkipsSourcePeer(t *testing.T) { rule := &nmdata.PolicyRule{ Protocol: string(PolicyRuleProtocolTCP), + Action: string(PolicyTrafficActionAccept), Ports: []string{"22"}, Bidirectional: true, }