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") }