ErrorResponse > 30000 bytes is misparsed as a pre-protocol plain-text error

[RaduBerinde]

Summary

Since v1.11.0 (PR #1249, commit 0ad30496), recvMessage treats any ErrorResponse whose payload length exceeds MaxErrlen (30000) as a pre-protocol plain-text error. A legitimate v3 ErrorResponse larger than 30 KB is therefore never parsed into its fields — the caller gets a garbled string instead of the real error.

Affected code

conn.go (v1.12.3, ~line 1088):

// libpq checks "if ErrorResponse && (msgLength < 8 || msgLength > MAX_ERRLEN)" ...
if t == proto.ErrorResponse && (n < 4 || n > proto.MaxErrlen) {
    msg, _ := cn.buf.ReadString('\x00')
    return 0, fmt.Errorf("pq: server error: %s%s", string(x[1:]), strings.TrimSuffix(msg, "\x00"))
}

MaxErrlen = 30_000 (internal/proto/proto.go).

Why it's wrong

The guard from PR #1249 is correct for its intended case (a backend that can't fork sends Ecould not fork… as plain text before the protocol handshake).

But libpq only treats msgLength > 30000 as a pre-protocol error during connection establishment (fe-connect.c, PQconnectPoll → CONNECTION_AWAITING_RESPONSE), where the comment notes it's coping with a pre-3.0-protocol server. In steady-state message parsing (fe-protocol3.c, pqParseInput3) there is a 30000 sanity check, but ErrorResponse is explicitly exempt via VALID_LONG_MESSAGE_TYPE (alongside DataRow, CopyData, NoticeResponse, etc.):

#define VALID_LONG_MESSAGE_TYPE(id) \
    ((id) == PqMsg_CopyData || \
     (id) == PqMsg_DataRow || \
     (id) == PqMsg_ErrorResponse || \
     (id) == PqMsg_FunctionCallResponse || \
     (id) == PqMsg_NoticeResponse || \
     (id) == PqMsg_NotificationResponse || \
     (id) == PqMsg_RowDescription)

...

if (msgLength > 30000 && \!VALID_LONG_MESSAGE_TYPE(id))
{
    handleSyncLoss(conn, id, msgLength);
    return;
}

So libpq deliberately allows large error messages once the protocol is established; lib/pq instead applies the startup-only cap to every ErrorResponse, mangling valid errors > 30 KB.

The returned string is also nonsense: string(x[1:]) is the 4 raw big-endian length bytes, so the error reads like pq: server error: <binary length><start of body> (e.g. pq: server error: �NSERROR, where �N are length bytes and SERROR is the start of the Severity field).

Reproduction

Against any PostgreSQL server (no special setup):

db, _ := sql.Open("postgres", connStr)
// Raise an error whose message field alone is ~40 KB.
_, err := db.Exec(`DO $$ BEGIN RAISE EXCEPTION '%', repeat('x', 40000); END $$;`)
fmt.Println(err)
// v1.10.9:  pq: xxxxxxxx... (the real message)
// v1.11.0+: pq: server error: <garbage>   (fields never parsed)

We hit this in CockroachDB, whose schema-changer errors embed graphviz URLs and routinely exceed ~50 KB.

Suggested fix

Scope the > MaxErrlen heuristic to the connection-startup path only (mirroring libpq), or drop the upper bound in steady-state recvMessage and rely on the length prefix. The < 8/< 4 lower-bound and OOM protection for the pre-handshake case can be preserved.

Affected versions: v1.11.0 – v1.12.3 (latest).

[RaduBerinde]

Severity: this corrupts the connection, it doesn't just garble the error

Digging further, the MaxErrlen branch in recvMessage does more than return a wrong string — it leaves the connection desynchronized and silently returns it to the pool. Trace (line numbers from v1.12.3):

1. A query sets inProgress = true via CompareAndSwap(false, true) (conn.go:895/950).
2. inProgress is only reset on ReadyForQuery, and that reset lives inside recvMessage (conn.go:1079):

   if proto.ResponseCode(t) == proto.ReadyForQuery {
       cn.inProgress.Store(false)
   }

3. The MaxErrlen branch returns early, before that point (conn.go:1088):

   if t == proto.ErrorResponse && (n < 4 || n > proto.MaxErrlen) {
       msg, _ := cn.buf.ReadString('\x00')   // reads only the first NUL-terminated field
       return 0, fmt.Errorf("pq: server error: %s%s", string(x[1:]), strings.TrimSuffix(msg, "\x00"))
   }

   It consumes the 5-byte header plus a single field, leaving the rest of the ErrorResponse body and the following ReadyForQuery unread in cn.buf. The wire is now out of sync and inProgress is never cleared.
4. The error is a plain fmt.Errorf, so handleError (error.go:292) hits case error: and only maps to driver.ErrBadConn for io.EOF / io.ErrUnexpectedEOF / TLS handshake failure. None apply, so the connection is not marked bad and database/sql keeps it in the pool.
5. The next use of that connection fails immediately at the CompareAndSwap:

   pq: there is already a query being processed on this connection
   

   …because inProgress is still true (with stale bytes queued behind it). Since the connection is pooled, this can also surface on an unrelated goroutine that later borrows it.

The original PR's intended trigger is a pre-handshake error during connection establishment, where the connection is discarded anyway — so the undrained buffer and leaked inProgress flag are harmless there. The regression is in applying that same early-return path to a steady-state ErrorResponse.

Minimal repro extends the earlier one — the second statement is the tell:

db, _ := sql.Open("postgres", connStr)
_, err := db.Exec(`DO $$ BEGIN RAISE EXCEPTION '%', repeat('x', 40000); END $$;`)
fmt.Println("1:", err) // pq: server error: <garbage>
_, err = db.Exec(`SELECT 1`)
fmt.Println("2:", err) // pq: there is already a query being processed on this connection

So beyond the suggested fix, the branch should also drain through ReadyForQuery (or mark the connection driver.ErrBadConn) so a poisoned connection isn't returned to the pool.