diff --git a/pgproto3/authentication_sasl.go b/pgproto3/authentication_sasl.go index 996b97d3..59650d4c 100644 --- a/pgproto3/authentication_sasl.go +++ b/pgproto3/authentication_sasl.go @@ -36,10 +36,11 @@ func (dst *AuthenticationSASL) Decode(src []byte) error { authMechanisms := src[4:] for len(authMechanisms) > 1 { idx := bytes.IndexByte(authMechanisms, 0) - if idx > 0 { - dst.AuthMechanisms = append(dst.AuthMechanisms, string(authMechanisms[:idx])) - authMechanisms = authMechanisms[idx+1:] + if idx == -1 { + return &invalidMessageFormatErr{messageType: "AuthenticationSASL", details: "unterminated string"} } + dst.AuthMechanisms = append(dst.AuthMechanisms, string(authMechanisms[:idx])) + authMechanisms = authMechanisms[idx+1:] } return nil diff --git a/pgproto3/command_complete.go b/pgproto3/command_complete.go index a19b906c..814027ca 100644 --- a/pgproto3/command_complete.go +++ b/pgproto3/command_complete.go @@ -18,8 +18,11 @@ func (*CommandComplete) Backend() {} // type identifier and 4 byte message length. func (dst *CommandComplete) Decode(src []byte) error { idx := bytes.IndexByte(src, 0) + if idx == -1 { + return &invalidMessageFormatErr{messageType: "CommandComplete", details: "unterminated string"} + } if idx != len(src)-1 { - return &invalidMessageFormatErr{messageType: "CommandComplete"} + return &invalidMessageFormatErr{messageType: "CommandComplete", details: "string terminated too early"} } dst.CommandTag = src[:idx] diff --git a/pgproto3/data_row.go b/pgproto3/data_row.go index 0bfe9a0d..4de77977 100644 --- a/pgproto3/data_row.go +++ b/pgproto3/data_row.go @@ -43,19 +43,19 @@ func (dst *DataRow) Decode(src []byte) error { return &invalidMessageFormatErr{messageType: "DataRow"} } - msgSize := int(int32(binary.BigEndian.Uint32(src[rp:]))) + valueLen := int(int32(binary.BigEndian.Uint32(src[rp:]))) rp += 4 // null - if msgSize == -1 { + if valueLen == -1 { dst.Values[i] = nil } else { - if len(src[rp:]) < msgSize { + if len(src[rp:]) < valueLen || valueLen < 0 { return &invalidMessageFormatErr{messageType: "DataRow"} } - dst.Values[i] = src[rp : rp+msgSize : rp+msgSize] - rp += msgSize + dst.Values[i] = src[rp : rp+valueLen : rp+valueLen] + rp += valueLen } } diff --git a/pgproto3/fuzz_test.go b/pgproto3/fuzz_test.go index 84ea8430..332596ab 100644 --- a/pgproto3/fuzz_test.go +++ b/pgproto3/fuzz_test.go @@ -4,22 +4,50 @@ import ( "bytes" "testing" + "github.com/jackc/pgx/v5/internal/pgio" "github.com/jackc/pgx/v5/pgproto3" "github.com/stretchr/testify/require" ) func FuzzFrontend(f *testing.F) { - testcases := [][]byte{ - {'Z', 0, 0, 0, 5}, + testcases := []struct { + msgType byte + msgLen uint32 + msgBody []byte + }{ + { + msgType: 'Z', + msgLen: 2, + msgBody: []byte{'I'}, + }, + { + msgType: 'Z', + msgLen: 5, + msgBody: []byte{'I'}, + }, } for _, tc := range testcases { - f.Add(tc) + f.Add(tc.msgType, tc.msgLen, tc.msgBody) } - f.Fuzz(func(t *testing.T, encodedMsg []byte) { + f.Fuzz(func(t *testing.T, msgType byte, msgLen uint32, msgBody []byte) { + // Prune any msgLen > len(msgBody) because they would hang the test waiting for more input. + if int(msgLen) > len(msgBody)+4 { + return + } + + // Prune any messages that are too long. + if msgLen > 128 || len(msgBody) > 128 { + return + } + r := &bytes.Buffer{} w := &bytes.Buffer{} fe := pgproto3.NewFrontend(r, w) + var encodedMsg []byte + encodedMsg = append(encodedMsg, msgType) + encodedMsg = pgio.AppendUint32(encodedMsg, msgLen) + encodedMsg = append(encodedMsg, msgBody...) _, err := r.Write(encodedMsg) require.NoError(t, err) diff --git a/pgproto3/notification_response.go b/pgproto3/notification_response.go index 03ce51e5..228e0dac 100644 --- a/pgproto3/notification_response.go +++ b/pgproto3/notification_response.go @@ -22,6 +22,10 @@ func (*NotificationResponse) Backend() {} func (dst *NotificationResponse) Decode(src []byte) error { buf := bytes.NewBuffer(src) + if buf.Len() < 4 { + return &invalidMessageFormatErr{messageType: "NotificationResponse", details: "too short"} + } + pid := binary.BigEndian.Uint32(buf.Next(4)) b, err := buf.ReadBytes(0) diff --git a/pgproto3/pgproto3.go b/pgproto3/pgproto3.go index a0333aa5..ef5a5489 100644 --- a/pgproto3/pgproto3.go +++ b/pgproto3/pgproto3.go @@ -46,10 +46,11 @@ func (e *invalidMessageLenErr) Error() string { type invalidMessageFormatErr struct { messageType string + details string } func (e *invalidMessageFormatErr) Error() string { - return fmt.Sprintf("%s body is invalid", e.messageType) + return fmt.Sprintf("%s body is invalid %s", e.messageType, e.details) } type writeError struct { diff --git a/pgproto3/testdata/fuzz/FuzzFrontend/39c5e864da4707fc15fea48f7062d6a07796fdc43b33e0ba9dbd7074a0211fa6 b/pgproto3/testdata/fuzz/FuzzFrontend/39c5e864da4707fc15fea48f7062d6a07796fdc43b33e0ba9dbd7074a0211fa6 new file mode 100644 index 00000000..d1c612d3 --- /dev/null +++ b/pgproto3/testdata/fuzz/FuzzFrontend/39c5e864da4707fc15fea48f7062d6a07796fdc43b33e0ba9dbd7074a0211fa6 @@ -0,0 +1,4 @@ +go test fuzz v1 +byte('A') +uint32(5) +[]byte("0") diff --git a/pgproto3/testdata/fuzz/FuzzFrontend/65d91093341a68b16f04605e392b0501847a9b35d3857e67872046dbdc04913e b/pgproto3/testdata/fuzz/FuzzFrontend/65d91093341a68b16f04605e392b0501847a9b35d3857e67872046dbdc04913e deleted file mode 100644 index 4db40929..00000000 --- a/pgproto3/testdata/fuzz/FuzzFrontend/65d91093341a68b16f04605e392b0501847a9b35d3857e67872046dbdc04913e +++ /dev/null @@ -1,2 +0,0 @@ -go test fuzz v1 -[]byte("0\x00\x00\x00\x02") diff --git a/pgproto3/testdata/fuzz/FuzzFrontend/9b06792b1aaac8a907dbfa04d526ae14326c8573b7409032caac8461e83065f7 b/pgproto3/testdata/fuzz/FuzzFrontend/9b06792b1aaac8a907dbfa04d526ae14326c8573b7409032caac8461e83065f7 new file mode 100644 index 00000000..763b70ae --- /dev/null +++ b/pgproto3/testdata/fuzz/FuzzFrontend/9b06792b1aaac8a907dbfa04d526ae14326c8573b7409032caac8461e83065f7 @@ -0,0 +1,4 @@ +go test fuzz v1 +byte('D') +uint32(21) +[]byte("00\xb300000000000000") diff --git a/pgproto3/testdata/fuzz/FuzzFrontend/a661fb98e802839f0a7361160fbc6e28794612a411d00bde104364ee281c4214 b/pgproto3/testdata/fuzz/FuzzFrontend/a661fb98e802839f0a7361160fbc6e28794612a411d00bde104364ee281c4214 new file mode 100644 index 00000000..3d995c28 --- /dev/null +++ b/pgproto3/testdata/fuzz/FuzzFrontend/a661fb98e802839f0a7361160fbc6e28794612a411d00bde104364ee281c4214 @@ -0,0 +1,4 @@ +go test fuzz v1 +byte('C') +uint32(4) +[]byte("0") diff --git a/pgproto3/testdata/fuzz/FuzzFrontend/fc98dcd487a5173b38763a5f7dd023933f3a86ab566e3f2b091eb36248107eb4 b/pgproto3/testdata/fuzz/FuzzFrontend/fc98dcd487a5173b38763a5f7dd023933f3a86ab566e3f2b091eb36248107eb4 new file mode 100644 index 00000000..45f0ba81 --- /dev/null +++ b/pgproto3/testdata/fuzz/FuzzFrontend/fc98dcd487a5173b38763a5f7dd023933f3a86ab566e3f2b091eb36248107eb4 @@ -0,0 +1,4 @@ +go test fuzz v1 +byte('R') +uint32(13) +[]byte("\x00\x00\x00\n0\x12\xebG\x8dI']G\xdac\x95\xb7\x18\xb0\x02\xe8m\xc2\x00\xef\x03\x12\x1b\xbdj\x10\x9f\xf9\xeb\xb8")