diff --git a/util.go b/util.go index 015a44ec..4bc1ad8b 100644 --- a/util.go +++ b/util.go @@ -66,11 +66,12 @@ func parseString(in []byte) (out string, rest []byte, ok bool) { return } length := binary.BigEndian.Uint32(in) - if uint32(len(in)) < 4+length { + in = in[4:] + if uint32(len(in)) < length { return } - out = string(in[4 : 4+length]) - rest = in[4+length:] + out = string(in[:length]) + rest = in[length:] ok = true return } diff --git a/util_test.go b/util_test.go new file mode 100644 index 00000000..3c43b293 --- /dev/null +++ b/util_test.go @@ -0,0 +1,44 @@ +package ssh + +import ( + "encoding/binary" + "testing" +) + +func TestParseStringValid(t *testing.T) { + in := make([]byte, 4+5) + binary.BigEndian.PutUint32(in, 5) + copy(in[4:], []byte("hello")) + out, rest, ok := parseString(in) + if !ok { + t.Fatal("expected ok for a well-formed string") + } + if out != "hello" { + t.Fatalf("got %q, want %q", out, "hello") + } + if len(rest) != 0 { + t.Fatalf("got %d bytes of rest, want 0", len(rest)) + } +} + +func TestParseStringLengthTooLarge(t *testing.T) { + // length prefix of 0xFFFFFFFF. In the old code 4+length overflowed the + // uint32 to 3, defeating the bounds check and slicing in[4:3]. + in := []byte{0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00} + out, rest, ok := parseString(in) + if ok { + t.Fatalf("expected ok=false for oversized length, got out=%q", out) + } + if rest != nil { + t.Fatalf("expected nil rest, got %v", rest) + } +} + +func TestParsePtyRequestOversizedTerm(t *testing.T) { + // A pty-req whose term-string length is 0xFFFFFFFF must be rejected, not panic. + payload := []byte{0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00} + _, ok := parsePtyRequest(payload) + if ok { + t.Fatal("expected ok=false for malformed pty-req") + } +}