Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion driver/pgdriver/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,12 @@ const (
)

func readColumnValue(rd *reader, dataType int32, dataLen int) (any, error) {
if dataLen == -1 {
if dataLen < 0 {
if dataLen == -1 {
return nil, nil
}
return nil, errInvalidMessageLength // TODO or nil?
}
return nil, nil
}

Expand Down Expand Up @@ -61,6 +66,13 @@ func readColumnValue(rd *reader, dataType int32, dataLen int) (any, error) {
return readBytesCol(rd, dataLen)
}

if dataLen < 0 {
// dataLen == -1 signals SQL NULL; any other negative value is invalid
// and must not reach make([]byte, dataLen) (would panic). dataLen == 0
// is fine and keeps its previous behavior (empty value).
return nil, nil
}

b := make([]byte, dataLen)
if _, err := io.ReadFull(rd, b); err != nil {
return nil, err
Expand Down
8 changes: 8 additions & 0 deletions driver/pgdriver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,14 @@ func (cn *Conn) reader(ctx context.Context, timeout time.Duration) *reader {
}

func (cn *Conn) write(ctx context.Context, wb *writeBuffer) error {
// Refuse to send a message that failed to build (e.g. it exceeded the
// protocol size limit); sending it would desync the connection.
if wb.err != nil {
err := wb.err
wb.Reset()
return err
}

cn.setWriteDeadline(ctx, -1)

n, err := cn.netConn.Write(wb.Bytes)
Expand Down
11 changes: 11 additions & 0 deletions driver/pgdriver/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,15 @@ func isBadConn(err error, allowTimeout bool) bool {

var (
errRequiresParameter = errors.New("pgdriver: requires that standard_conforming_strings=on and client_encoding=UTF8")

// errMessageTooLarge is returned instead of sending a message whose length
// prefix would overflow the protocol's 32-bit size field. Without this guard
// the length wraps and the server desyncs, which can be abused for SQL
// injection (cf. jackc/pgx CVE-2024-27304).
errMessageTooLarge = errors.New("pgdriver: message size exceeds the maximum of 4 GiB")

// errInvalidMessageLength is returned when the server sends a message whose
// length field is smaller than the 4-byte length prefix itself (which would
// otherwise lead to a negative-length slice/allocation panic).
errInvalidMessageLength = errors.New("pgdriver: server sent a message with an invalid length")
)
84 changes: 84 additions & 0 deletions driver/pgdriver/message_size_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package pgdriver

import (
"bytes"
"context"
"database/sql/driver"
"encoding/binary"
"errors"
"math"
"testing"
)

func TestFinishMessage_TooLargeSetsError(t *testing.T) {
orig := maxMessageSize
maxMessageSize = 8
defer func() { maxMessageSize = orig }()

wb := newWriteBuffer()
wb.StartMessage(queryMsg)
wb.WriteString("some query that exceeds the tiny test limit")
wb.FinishMessage()

if !errors.Is(wb.err, errMessageTooLarge) {
t.Fatalf("wb.err = %v, want errMessageTooLarge", wb.err)
}
}

func TestConnWrite_RefusesOversizedMessage(t *testing.T) {
wb := newWriteBuffer()
wb.setErr(errMessageTooLarge)

// wb.err is set, so write must return it without touching the (nil) netConn.
if err := (&Conn{}).write(context.Background(), wb); !errors.Is(err, errMessageTooLarge) {
t.Fatalf("write() = %v, want errMessageTooLarge", err)
}
}

func TestFinishParam_TooLargeSetsError(t *testing.T) {
orig := maxMessageSize
maxMessageSize = 4
defer func() { maxMessageSize = orig }()

wb := newWriteBuffer()
wb.StartParam()
wb.Write([]byte("larger than the limit"))
wb.FinishParam()

if !errors.Is(wb.err, errMessageTooLarge) {
t.Fatalf("wb.err = %v, want errMessageTooLarge", wb.err)
}
}

func TestWriteBindExecute_TooManyParams(t *testing.T) {
args := make([]driver.NamedValue, math.MaxInt16+1)
err := writeBindExecute(context.Background(), &Conn{}, "", args)
if err == nil {
t.Fatal("writeBindExecute with > MaxInt16 params: got nil error, want an error")
}
}

func TestReadMessageType_RejectsShortLength(t *testing.T) {
// type byte + 4-byte length field of 0 (< 4, invalid).
buf := []byte{'X', 0, 0, 0, 0}
rd := newReader(bytes.NewReader(buf), 1024)
if _, _, err := readMessageType(rd); !errors.Is(err, errInvalidMessageLength) {
t.Fatalf("readMessageType short length: err = %v, want errInvalidMessageLength", err)
}

// A valid length (>= 4) is accepted and the body length is length-4.
valid := []byte{'X', 0, 0, 0, 0}
binary.BigEndian.PutUint32(valid[1:], 10)
rd = newReader(bytes.NewReader(valid), 1024)
c, n, err := readMessageType(rd)
if err != nil || c != 'X' || n != 6 {
t.Fatalf("readMessageType valid: (%q, %d, %v), want ('X', 6, nil)", c, n, err)
}
}

func TestReadTemp_RejectsNegative(t *testing.T) {
rd := newReader(bytes.NewReader(nil), 1024)
if _, err := rd.ReadTemp(-1); !errors.Is(err, errInvalidMessageLength) {
t.Fatalf("ReadTemp(-1) = %v, want errInvalidMessageLength", err)
}
}
13 changes: 13 additions & 0 deletions driver/pgdriver/proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ func newReader(r io.Reader, size int) *reader {
}

func (r *reader) ReadTemp(n int) ([]byte, error) {
if n < 0 {
return nil, errInvalidMessageLength
}
if n <= len(r.buf) {
b := r.buf[:n]
_, err := io.ReadFull(r.Reader, b)
Expand Down Expand Up @@ -827,6 +830,10 @@ func writeBindExecute(ctx context.Context, cn *Conn, name string, args []driver.
wb := getWriteBuffer()
defer putWriteBuffer(wb)

if len(args) > math.MaxInt16 {
return fmt.Errorf("pgdriver: too many bind parameters: %d (max %d)", len(args), math.MaxInt16)
}

wb.StartMessage(bindMsg)
wb.WriteString("")
wb.WriteString(name)
Expand Down Expand Up @@ -1020,6 +1027,12 @@ func readMessageType(rd *reader) (byte, int, error) {
if err != nil {
return 0, 0, err
}
if l < 4 {
// The length field includes its own 4 bytes, so it can never be < 4.
// A smaller value would yield a negative body length and panic callers
// that allocate/slice with it.
return 0, 0, errInvalidMessageLength
}
return c, int(l) - 4, nil
}

Expand Down
36 changes: 32 additions & 4 deletions driver/pgdriver/write_buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,16 @@ package pgdriver
import (
"encoding/binary"
"io"
"math"
"sync"
)

// maxMessageSize is the largest value that fits in the protocol's 32-bit message
// length field. It is an int64 (so the constant does not overflow int on 32-bit
// platforms) and a var (not a const) so tests can lower it. Do not mutate it
// outside of tests.
var maxMessageSize int64 = math.MaxUint32

var wbPool = sync.Pool{
New: func() any {
return newWriteBuffer()
Expand All @@ -27,6 +34,11 @@ type writeBuffer struct {

msgStart int
paramStart int

// err records the first fatal error (e.g. a message that exceeds the
// protocol size limit). It is checked before the buffer is sent so an
// oversized/overflowing message is never written to the wire.
err error
}

func newWriteBuffer() *writeBuffer {
Expand All @@ -37,6 +49,14 @@ func newWriteBuffer() *writeBuffer {

func (b *writeBuffer) Reset() {
b.Bytes = b.Bytes[:0]
b.err = nil
}

// setErr records the first error seen while building a message.
func (b *writeBuffer) setErr(err error) {
if b.err == nil {
b.err = err
}
}

func (b *writeBuffer) StartMessage(c byte) {
Expand All @@ -50,8 +70,12 @@ func (b *writeBuffer) StartMessage(c byte) {
}

func (b *writeBuffer) FinishMessage() {
binary.BigEndian.PutUint32(
b.Bytes[b.msgStart:], uint32(len(b.Bytes)-b.msgStart))
n := len(b.Bytes) - b.msgStart
if int64(n) > maxMessageSize {
b.setErr(errMessageTooLarge)
return
}
binary.BigEndian.PutUint32(b.Bytes[b.msgStart:], uint32(n))
}

func (b *writeBuffer) Query() []byte {
Expand All @@ -64,8 +88,12 @@ func (b *writeBuffer) StartParam() {
}

func (b *writeBuffer) FinishParam() {
binary.BigEndian.PutUint32(
b.Bytes[b.paramStart:], uint32(len(b.Bytes)-b.paramStart-4))
n := len(b.Bytes) - b.paramStart - 4
if int64(n) > maxMessageSize {
b.setErr(errMessageTooLarge)
return
}
binary.BigEndian.PutUint32(b.Bytes[b.paramStart:], uint32(n))
}

var nullParamLength = int32(-1)
Expand Down