fix(rpc): enforce magic ff05ff05 on IOStream init

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>
This commit is contained in:
naiba
2026-05-18 15:17:18 +00:00
co-authored by naiba/CloudCode
parent 710a2c731c
commit 26cf9b3fa6
3 changed files with 64 additions and 2 deletions
+47
View File
@@ -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)
}
}
}