diff --git a/modules/mssql/connection.go b/modules/mssql/connection.go index bdd8fb04..354c71c0 100644 --- a/modules/mssql/connection.go +++ b/modules/mssql/connection.go @@ -443,16 +443,20 @@ func decodePreloginOptions(body []byte) (result *PreloginOptions, rest []byte, e return nil, nil, ErrInvalidData } token := PreloginOptionToken(cursor[0]) - offset := binary.BigEndian.Uint16(cursor[1:3]) - length := binary.BigEndian.Uint16(cursor[3:5]) - if len(body) < int(offset+length) { + // Widen to int before adding: offset and length are attacker-controlled + // uint16s, so offset+length can overflow uint16 and wrap to a small + // value, defeating the bounds check below and causing an out-of-range + // slice panic on the following line. + offset := int(binary.BigEndian.Uint16(cursor[1:3])) + length := int(binary.BigEndian.Uint16(cursor[3:5])) + if len(body) < offset+length { return nil, nil, ErrInvalidData } options[token] = body[offset : offset+length] - if int(offset+length) > max { + if offset+length > max { // max points to the byte after the last byte consumed in body - max = int(offset + length) + max = offset + length } cursor = cursor[5:] } diff --git a/modules/mssql/connection_test.go b/modules/mssql/connection_test.go new file mode 100644 index 00000000..10ec8629 --- /dev/null +++ b/modules/mssql/connection_test.go @@ -0,0 +1,40 @@ +package mssql + +import ( + "errors" + "testing" +) + +// TestDecodePreloginOptionsOverflow ensures a PRELOGIN option whose +// offset+length overflows a uint16 is rejected as invalid data rather than +// panicking with an out-of-range slice. A malicious MSSQL server controls +// these fields, so the bounds check must not be defeated by integer overflow. +func TestDecodePreloginOptionsOverflow(t *testing.T) { + // token=0x00, offset=0xFFFF, length=0x0002 (offset+length wraps to 1 in + // uint16 arithmetic), terminator=0xff. + body := []byte{0x00, 0xFF, 0xFF, 0x00, 0x02, 0xFF} + opts, rest, err := decodePreloginOptions(body) + if !errors.Is(err, ErrInvalidData) { + t.Fatalf("expected ErrInvalidData, got opts=%v rest=%v err=%v", opts, rest, err) + } +} + +// TestDecodePreloginOptionsValid confirms a well-formed PRELOGIN body still +// decodes correctly after the overflow-hardening change. +func TestDecodePreloginOptionsValid(t *testing.T) { + // One option: token=0x00, offset=0x0006 (points just past the 6-byte + // header: 5-byte option entry + 0xff terminator), length=0x0002; followed + // by the terminator and the 2 value bytes. + body := []byte{0x00, 0x00, 0x06, 0x00, 0x02, 0xFF, 0xAB, 0xCD} + opts, _, err := decodePreloginOptions(body) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + got, ok := (*opts)[PreloginOptionToken(0x00)] + if !ok { + t.Fatalf("expected token 0x00 to be present, got %v", *opts) + } + if want := []byte{0xAB, 0xCD}; string(got) != string(want) { + t.Fatalf("expected value %v, got %v", want, got) + } +}