Reject negative Postgres binary field lengths

[marknefedov]

Problem

PostgresBinaryParser::ReadValue treats -1 as NULL but lets every other negative field length through:

https://github.com/duckdb/duckdb-postgres/blob/main/src/postgres_binary_parser.cpp#L154

ReadString takes an idx_t, so -2 arrives as 18446744073709551614, and the bounds check adds it to buffer_ptr before comparing. That overflows the pointer and wraps back below end, so the check passes:

value_len=-2 -> string_length=18446744073709551614
old check  (buffer_ptr + string_length > end): PASSES -> out of bounds read
new check  (string_length > idx_t(end - buffer_ptr)): THROWS

The length then reaches StringVector::AddStringOrBlob unchanged. Reachable from read_postgres_binary on a corrupt or hand-crafted file.

Fix

• Reject any negative length other than -1 in ReadValue.
• Bound-check ReadString against the bytes remaining rather than via pointer arithmetic, so a corrupt length cannot overflow past the guard.
• Fix the JSONB branch separately: it decrements value_len after the NULL check, so a zero length underflowed to -1 and hit the same path. Rejecting < -1 alone does not cover this.

This also makes the existing value_len < sizeof(uint16_t) * 4 check in the DECIMAL branch meaningful — it compares a signed value_len against a size_t, so a negative length used to wrap to a huge unsigned value and pass.

Test

Two 27 byte fixtures under test/data/, read by test/sql/misc/postgres_binary_read_corrupt.test:

• postgres_binary_negative_length.bin — field length -2, checked against VARCHAR, BLOB and INTEGER columns
• postgres_binary_oversized_length.bin — field length 1000 in a 27 byte file, locking in the ReadString bounds check

Known gap, not addressed here

Fixed width types still validate their length only with D_ASSERT, which is compiled out in release builds. Those all go through the bounds-checked ReadInteger so they cannot read out of bounds, but a wrong length is silently misparsed rather than rejected. That felt like a separate change; happy to fold it in if you would prefer one pass over the whole parser.

Notes

Draft — not built locally, relying on CI. The overflow characterisation above comes from a standalone harness, not from a live run.