From 2de21b2b6bb374f5b99c4ee0fdbb692285ee7c2b Mon Sep 17 00:00:00 2001
From: yuanyuanxiang <962914132@qq.com>
Date: Tue, 13 Oct 2026 16:19:39 +0200
Subject: [PATCH] Fix: Bind screen/terminal sub-conns only to the device's
 source IP

A screen or terminal sub-connection was keyed solely by a MasterID that any
TCP peer may supply, so any internet host could claim an online device's
MasterID and bind a sub-conn in its place, hijacking the live screen stream
or a pending shell session. The ConnAuth signature cannot be verified on the
server because its key lives in a vendored client library, so the trust
anchor here is the source IP: sub-conns are always opened device-to-server
from the same host as the login conn. BindScreenConn and BindTerminalConn
now reject a sub-conn whose peer IP differs from the device's recorded
main-connection PeerIP.

Co-Authored-By: deepseek-v4-pro
---
 server/go/hub/hub.go      | 18 +++++++++++++
 server/go/hub/hub_test.go | 54 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/server/go/hub/hub.go b/server/go/hub/hub.go
index 22c3e61..fae62df 100644
--- a/server/go/hub/hub.go
+++ b/server/go/hub/hub.go
@@ -299,6 +299,17 @@ func (h *Hub) BindScreenConn(deviceID string, ctx *connection.Context) bool {
 		h.mu.Unlock()
 		return false
 	}
+	// Bind only sub-connections that originate from the device's own main
+	// connection source IP. Sub-conns are always opened device→server from
+	// the same host as the login conn, so a source-IP mismatch means the
+	// caller is not that device. This is the trust anchor for screen/terminal
+	// sub-conns: the ConnAuth signature key lives in a vendored client
+	// library and cannot be verified here, so without this check any internet
+	// host could claim an online device's MasterID and hijack its stream.
+	if d.PeerIP != "" && ctx.GetPeerIP() != d.PeerIP {
+		h.mu.Unlock()
+		return false
+	}
 	old := d.screenConn
 	d.screenConn = ctx
 	// The cached resolution and last IDR belong to the old encoder's
@@ -693,6 +704,13 @@ func (h *Hub) BindTerminalConn(deviceID string, ctx *connection.Context, isPTY b
 		h.mu.Unlock()
 		return false
 	}
+	// Same trust anchor as BindScreenConn: a terminal sub-conn must come from
+	// the device's own main-connection source IP, otherwise any internet host
+	// could claim the pending session and hijack the shell.
+	if d.PeerIP != "" && ctx.GetPeerIP() != d.PeerIP {
+		h.mu.Unlock()
+		return false
+	}
 	d.terminalConn = ctx
 	d.terminalIsPTY = isPTY
 	d.terminalPending = false
diff --git a/server/go/hub/hub_test.go b/server/go/hub/hub_test.go
index 8df9c8d..38fc4dd 100644
--- a/server/go/hub/hub_test.go
+++ b/server/go/hub/hub_test.go
@@ -143,6 +143,60 @@ func TestHubRegisterOverwrites(t *testing.T) {
 	}
 }
 
+// ctxFromIP builds a *connection.Context whose source IP resolves to ip.
+// BindScreenConn/BindTerminalConn compare this against Device.PeerIP.
+func ctxFromIP(ip string) *connection.Context {
+	return &connection.Context{RemoteAddr: ip + ":54321"}
+}
+
+// Sub-conns (screen/terminal) are opened device→server from the same host as
+// the login conn. A caller claiming a MasterID from a different source IP is
+// not that device and must not be able to bind, replace, or hijack the stream.
+func TestHubBindScreenConnIPBinding(t *testing.T) {
+	h := New()
+	h.Register(&Device{ID: "dev", PeerIP: "10.0.0.1"}, stubCtx())
+
+	// First bind from the device's own IP succeeds.
+	if !h.BindScreenConn("dev", ctxFromIP("10.0.0.1")) {
+		t.Fatal("same-IP screen bind should succeed")
+	}
+	// A second conn claiming the same MasterID from a foreign IP is refused,
+	// so it cannot displace the legitimate stream.
+	if h.BindScreenConn("dev", ctxFromIP("203.0.113.9")) {
+		t.Fatal("foreign-IP screen bind should be rejected")
+	}
+
+	// A device that has never bound a screen conn is equally protected.
+	h.Register(&Device{ID: "fresh", PeerIP: "10.0.0.2"}, stubCtx())
+	if h.BindScreenConn("fresh", ctxFromIP("203.0.113.9")) {
+		t.Fatal("foreign-IP first screen bind should be rejected")
+	}
+}
+
+func TestHubBindTerminalConnIPBinding(t *testing.T) {
+	h := New()
+	h.Register(&Device{ID: "dev", PeerIP: "10.0.0.1"}, stubCtx())
+
+	openPending := func(id string) {
+		h.mu.Lock()
+		h.devices[id].terminalPending = true
+		h.mu.Unlock()
+	}
+	openPending("dev")
+
+	// Same-IP terminal bind succeeds and consumes the pending slot.
+	if !h.BindTerminalConn("dev", ctxFromIP("10.0.0.1"), true) {
+		t.Fatal("same-IP terminal bind should succeed")
+	}
+
+	// A second device with a pending session cannot be claimed from elsewhere.
+	h.Register(&Device{ID: "dev2", PeerIP: "10.0.0.2"}, stubCtx())
+	openPending("dev2")
+	if h.BindTerminalConn("dev2", ctxFromIP("203.0.113.9"), true) {
+		t.Fatal("foreign-IP terminal bind should be rejected")
+	}
+}
+
 // Race detector should not fire under `go test -race ./hub/...`.
 func TestHubConcurrent(t *testing.T) {
 	h := New()