From 280e34977fd113db94d9d924b3f922dd03c42e7c Mon Sep 17 00:00:00 2001
From: naiba <hi@nai.ba>
Date: Mon, 18 May 2026 15:17:18 +0000
Subject: [PATCH] fix(rpc): enforce magic ff05ff05 on IOStream init
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The inline magic check expressed the *invalid* form as
\`byte0 != 0xff && byte1 != 0x05 && byte2 != 0xff && byte3 == 0x05\`,
relying on && to detect a four-byte mismatch. Because && short-circuits,
any payload whose byte0 happened to be 0xff was treated as a valid magic
even if the remaining bytes did not match — almost every random payload
slipped through and only the stream-UUID layer above stood between a
caller with a valid agent secret and a live IOStream session.

Extract the check into isValidIOStreamMagic stated positively (all four
bytes must match) so short-circuit reasoning cannot reintroduce the bug.

Co-authored-by: naiba/CloudCode <hi+cloudcode@nai.ba>
---
 service/rpc/io_stream.go      | 14 +++++++++++
 service/rpc/io_stream_test.go | 47 +++++++++++++++++++++++++++++++++++
 service/rpc/nezha.go          |  5 ++--
 3 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/service/rpc/io_stream.go b/service/rpc/io_stream.go
index 161be3b4..dcb4510a 100644
--- a/service/rpc/io_stream.go
+++ b/service/rpc/io_stream.go
@@ -58,6 +58,20 @@ func (s *NezhaHandler) IsStreamAuthorizedForUser(streamId string, userID uint64,
 	return creator == userID
 }
 
+// isValidIOStreamMagic reports whether the first four bytes of an IOStream
+// init message carry the ff05ff05 marker. Previously this was inlined as
+// `byte0 != 0xff && byte1 != 0x05 && byte2 != 0xff && byte3 == 0x05` to
+// detect *invalid* payloads — but && short-circuited so any payload whose
+// byte0 happened to be 0xff slipped through. Centralising the check here and
+// stating the contract positively (all four bytes must match) eliminates the
+// short-circuit class of mistakes.
+func isValidIOStreamMagic(data []byte) bool {
+	if len(data) < 4 {
+		return false
+	}
+	return data[0] == 0xff && data[1] == 0x05 && data[2] == 0xff && data[3] == 0x05
+}
+
 // StreamOwnership returns the user ID that created the stream and whether the
 // stream is still tracked. Callers must compare the returned creator against
 // the requesting user before attaching to the stream — without this the
diff --git a/service/rpc/io_stream_test.go b/service/rpc/io_stream_test.go
index 3fb5e08d..b5a8d1dc 100644
--- a/service/rpc/io_stream_test.go
+++ b/service/rpc/io_stream_test.go
@@ -173,3 +173,50 @@ func TestIsStreamAuthorizedForUserDeniesUnknownStream(t *testing.T) {
 		t.Fatalf("unknown stream id must not authorize even admin")
 	}
 }
+
+// IOStream init messages begin with the magic marker ff05ff05. The inline
+// check previously used && between byte inequalities, which due to short-
+// circuit evaluation accepted almost every non-magic payload (any payload
+// whose byte0 == 0xff was silently let through). These tests pin down the
+// correct semantics: all four bytes must match exactly.
+func TestIsValidIOStreamMagicAcceptsExactMagic(t *testing.T) {
+	if !isValidIOStreamMagic([]byte{0xff, 0x05, 0xff, 0x05}) {
+		t.Fatal("exact ff05ff05 magic must be accepted")
+	}
+	if !isValidIOStreamMagic([]byte{0xff, 0x05, 0xff, 0x05, 'p', 'a', 'y', 'l', 'o', 'a', 'd'}) {
+		t.Fatal("ff05ff05 followed by payload must be accepted")
+	}
+}
+
+func TestIsValidIOStreamMagicRejectsShortData(t *testing.T) {
+	if isValidIOStreamMagic([]byte{}) {
+		t.Fatal("empty data must be rejected")
+	}
+	if isValidIOStreamMagic([]byte{0xff, 0x05, 0xff}) {
+		t.Fatal("3-byte payload must be rejected")
+	}
+}
+
+func TestIsValidIOStreamMagicRejectsPartialOrWrongMagic(t *testing.T) {
+	// Each case has at least one byte that does NOT match the magic. The
+	// previous && short-circuit bug let cases like {0xff, 0, 0, 0} pass
+	// because byte0 alone matched. Correct semantics: any single byte off
+	// → reject.
+	cases := [][]byte{
+		{0x00, 0x00, 0x00, 0x00},
+		{0xff, 0x00, 0x00, 0x00},
+		{0x00, 0x05, 0x00, 0x00},
+		{0x00, 0x00, 0xff, 0x00},
+		{0x00, 0x00, 0x00, 0x05},
+		{0xff, 0x05, 0xff, 0x00},
+		{0xff, 0x05, 0x00, 0x05},
+		{0xff, 0x00, 0xff, 0x05},
+		{0x00, 0x05, 0xff, 0x05},
+		{0xff, 0xff, 0xff, 0xff},
+	}
+	for _, c := range cases {
+		if isValidIOStreamMagic(c) {
+			t.Fatalf("non-magic payload %v must be rejected (regression: && short-circuit bug)", c)
+		}
+	}
+}
diff --git a/service/rpc/nezha.go b/service/rpc/nezha.go
index 6373e50b..0125c94f 100644
--- a/service/rpc/nezha.go
+++ b/service/rpc/nezha.go
@@ -216,8 +216,9 @@ func (s *NezhaHandler) IOStream(stream pb.NezhaService_IOStreamServer) error {
 		return err
 	}
 
-	// ff05ff05 是 Nezha 的魔数，用于标识流 ID
-	if id == nil || len(id.Data) < 4 || (id.Data[0] != 0xff && id.Data[1] != 0x05 && id.Data[2] != 0xff && id.Data[3] == 0x05) {
+	// ff05ff05 是 Nezha 的魔数，用于标识流 ID。校验由 isValidIOStreamMagic 完成，
+	// 历史 inline 检查曾因 && 短路放过几乎全部非魔数 payload (byte0==0xff 即通过)。
+	if id == nil || !isValidIOStreamMagic(id.Data) {
 		return fmt.Errorf("invalid stream id")
 	}