Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
72 changes: 65 additions & 7 deletions src/sql_jsc/postgres/DataCell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@

const MAX_ARRAY_NESTING_DEPTH: usize = 100;

// Microseconds in a day. PostgreSQL's TIME/TIMETZ range is [00:00:00, 24:00:00].
const USECS_PER_DAY: i64 = 86_400_000_000;

// PERF: `array_type` and `is_json_sub_array` are only used in value
// position (branch selectors), never type position. Profile if it shows up on a hot path.
fn parse_array(
Expand Down Expand Up @@ -684,9 +687,20 @@
} else {
let mut out: Box<[u8]> = vec![0u8; out_bytes].into_boxed_slice();
for i in 0..array_len {
// Wire layout per element for the 4-byte types this path
// supports (int4/float4): [elem_size length prefix][elem_size value]
let src_off = 20 + i * element_stride + (element_stride - elem_size);
// Wire layout per element: [int4 length prefix][value]. For the
// fixed-size types this path supports (int4/float4) the declared
// length must equal elem_size; validate before trusting the stride.
let len_off = 20 + i * element_stride;
let elem_len = i32::from_ne_bytes(
bytes[len_off..len_off + 4]
.try_into()
.expect("infallible: size matches"),
)
.swap_bytes();
if elem_len != elem_size as i32 {
return Err(AnyPostgresError::InvalidBinaryData);
}
let src_off = len_off + 4;
// `bytes.len() >= 20 + array_len*element_stride` was validated
// above; `out` has `array_len*elem_size` bytes. The trait's
// `from_unaligned_ne_bytes`/`write_unaligned_ne_bytes` are safe
Expand Down Expand Up @@ -778,7 +792,9 @@
}
}
T::float8 => {
if binary && bytes.len() == 8 {
if binary {
// Binary float8 is exactly 8 bytes; reject a short/long datum
// instead of falling through to the text parser (which yields NaN).
Ok(SQLDataCell::float8(parse_binary_float8(bytes)?))
} else {
Ok(SQLDataCell::float8(
Expand All @@ -787,7 +803,13 @@
}
}
T::float4 => {
if binary && bytes.len() == 4 {
if binary {
// Binary float4 is exactly 4 bytes. parse_binary_float4 goes
// through parse_binary_int4, which also accepts 1/2 bytes, so
// enforce the width here.
if bytes.len() != 4 {
return Err(AnyPostgresError::InvalidBinaryData);
}
Ok(SQLDataCell::float8(parse_binary_float4(bytes)? as f64))
} else {
Ok(SQLDataCell::float8(
Expand All @@ -812,16 +834,26 @@
T::jsonb | T::json => Ok(SQLDataCell::json(bytes)),
T::bool => {
if binary {
Ok(SQLDataCell::bool_(!bytes.is_empty() && bytes[0] == 1))
// Binary bool is exactly 1 byte valued 0 or 1.
if bytes.len() != 1 || bytes[0] > 1 {
return Err(AnyPostgresError::InvalidBinaryData);
}
Ok(SQLDataCell::bool_(bytes[0] == 1))
} else {
Ok(SQLDataCell::bool_(!bytes.is_empty() && bytes[0] == b't'))
}
}
tag @ (T::date | T::timestamp | T::timestamptz) => {
if bytes.is_empty() {
return Ok(SQLDataCell::null());
}
if binary && bytes.len() == 8 {
if binary {
// Binary timestamp/timestamptz is an 8-byte int64 of
// microseconds. A short/long datum is rejected rather than
// reinterpreted as text (which produces an Invalid Date).
if bytes.len() != 8 {
return Err(AnyPostgresError::InvalidBinaryData);
}

Check warning on line 856 in src/sql_jsc/postgres/DataCell.rs

View check run for this annotation

Claude / Claude Code Review

0-byte binary timestamp/time datum returns null instead of InvalidBinaryData

Minor consistency gap: the pre-existing `if bytes.is_empty() { return Ok(SQLDataCell::null()) }` runs before the new `bytes.len() != 8` guard here (and in the `T::time | T::timetz` arm), so a 0-byte binary timestamp still surfaces as `null` instead of `InvalidBinaryData` — unlike the bool arm, which this PR now rejects for len=0. Consider moving the empty check inside the text branch, or checking `binary` first.
Comment thread
robobun marked this conversation as resolved.
Outdated
match tag {
T::timestamptz => Ok(SQLDataCell::date_with_tz(
crate::postgres::types::date::from_binary(bytes),
Expand Down Expand Up @@ -863,6 +895,12 @@
// PostgreSQL sends time as microseconds since midnight in binary format
let microseconds = i64::from_ne_bytes(bytes[0..8].try_into().expect("infallible: size matches")).swap_bytes();

// Valid range is [0, 24h]; out-of-range microseconds would
// otherwise feed the C formatter and yield garbage strings.
if !(0..=USECS_PER_DAY).contains(&microseconds) {
return Err(AnyPostgresError::InvalidTimeFormat);
}

// Use C++ helper for formatting
let mut buffer = [0u8; 32];
let len = Postgres__formatTime(microseconds, &mut buffer, 32);
Expand All @@ -873,6 +911,10 @@
let microseconds = i64::from_ne_bytes(bytes[0..8].try_into().expect("infallible: size matches")).swap_bytes();
let tz_offset_seconds = i32::from_ne_bytes(bytes[8..12].try_into().expect("infallible: size matches")).swap_bytes();

if !(0..=USECS_PER_DAY).contains(&microseconds) {
return Err(AnyPostgresError::InvalidTimeFormat);
}

// Use C++ helper for formatting with timezone
let mut buffer = [0u8; 48];
let len = Postgres__formatTimeTz(microseconds, tz_offset_seconds, &mut buffer, 48);
Expand Down Expand Up @@ -1022,6 +1064,16 @@
dscale
);

// The wire format is the 8-byte header followed by exactly `ndigits`
// base-10000 digits (2 bytes each). Reject a negative count, a negative
// display scale, and any trailing bytes before trusting the header.
if ndigits < 0 || dscale < 0 {
return Err(err!("InvalidBuffer"));
}
if input.len() != 8 + (ndigits as usize) * 2 {
return Err(err!("InvalidBuffer"));
}

// Handle special cases
match sign {
0xC000 => return Ok(PGNummericString::Static(b"NaN")),
Expand Down Expand Up @@ -1249,6 +1301,12 @@
// construct directly, no transmute needed.
types::Tag(oid as types::short)
};
// A RowDescription format code of 1 (binary) on a type we have no
// binary decoder for must be rejected, not silently reinterpreted
// as text: the raw datum bytes would otherwise surface as a value.
if field.binary && !tag.is_binary_format_supported() {
return Err(AnyPostgresError::UnknownFormatCode);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment thread
robobun marked this conversation as resolved.
Outdated
*cell = if let Some(data) = optional_bytes {
from_bytes(
(field.binary || self.binary) && tag.is_binary_format_supported(),
Expand Down
198 changes: 198 additions & 0 deletions test/js/sql/postgres-binary-datum-validation.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
// Fault-injection test: requires a server that sends malformed binary-format
// datums, which a healthy Postgres never does on demand. DO NOT COPY THIS
// PATTERN for behavior a real server can produce. All wire-protocol frames come
// from test/js/sql/wire-frames.ts; do not inline Buffer.alloc frame building.
//
// A RowDescription can declare format=1 (binary) for a column, after which the
// server authors the datum bytes. Bun must validate each binary datum's length
// (and range) against the declared type before decoding, and reject a binary
// format code for a type it has no binary decoder for, instead of silently
// turning a wire-level violation into a plausible JS value.
import { SQL } from "bun";
import { expect, test } from "bun:test";
import {
listeningServer,
pgAuthenticationOk,
pgCommandComplete,
pgDataRow,
pgReadyForQuery,
pgRowDescription,
} from "./wire-frames";

// Big-endian integer encoders for assembling hostile *column payloads* (the
// datum bytes inside a DataRow); these are not wire frames.
const i16 = (n: number): Buffer => {
const b = Buffer.alloc(2);
b.writeInt16BE(n, 0);
return b;
};
const u16 = (n: number): Buffer => {
const b = Buffer.alloc(2);
b.writeUInt16BE(n, 0);
return b;
};
const i32 = (n: number): Buffer => {
const b = Buffer.alloc(4);
b.writeInt32BE(n, 0);
return b;
};
const i64 = (n: bigint): Buffer => {
const b = Buffer.alloc(8);
b.writeBigInt64BE(n, 0);
return b;
};

async function runMockQuery(columnBytes: Buffer, typeOid: number): Promise<unknown> {
const { port, server } = await listeningServer(socket => {
let startup = true;
socket.on("data", data => {
if (startup) {
startup = false;
socket.write(Buffer.concat([pgAuthenticationOk(), pgReadyForQuery()]));
return;
}
if (data[0] !== 0x51 /* 'Q' */) return;
socket.write(
Buffer.concat([
pgRowDescription([{ name: "c", typeOid, format: 1 /* binary */ }]),
pgDataRow([columnBytes]),
pgCommandComplete("SELECT 1"),
pgReadyForQuery(),
]),
);
});
socket.on("error", () => {});
});

const sql = new SQL({
url: `postgres://u@127.0.0.1:${port}/db`,
max: 1,
idleTimeout: 5,
connectionTimeout: 5,
});

try {
return await sql`select c`.simple();
} finally {
await sql.close().catch(() => {});
await new Promise<void>(r => server.close(() => r()));
}
}

const BOOL = 16;
const INT4_ARRAY = 1007;
const FLOAT8 = 701;
const TIME = 1083;
const TIMESTAMP = 1114;
const NUMERIC = 1700;
const UUID = 2950;
const INT4 = 23;

// Binary numeric header — PostgreSQL numeric_send(): Int16 ndigits, Int16
// weight, uint16 sign, uint16 dscale, then ndigits Int16 base-10000 groups.
function numericHeader(ndigits: number, weight: number, sign: number, dscale: number): Buffer {
return Buffer.concat([i16(ndigits), i16(weight), u16(sign), u16(dscale)]);
}

const malformed: { name: string; oid: number; col: Buffer; code: RegExp }[] = [
{
name: "float8 with 4-byte datum",
oid: FLOAT8,
col: i32(0),
code: /ERR_POSTGRES_INVALID_BINARY_DATA/,
},

Check warning on line 103 in test/js/sql/postgres-binary-datum-validation.test.ts

View check run for this annotation

Claude / Claude Code Review

float4 binary length validation has no test coverage

The `malformed` table has no `float4` case, even though the PR adds an explicit `bytes.len() != 4` guard for `T::float4` and lists it in the fix table. Consider adding a row like `{ name: "float4 with 2-byte datum", oid: 700, col: i16(0), code: /ERR_POSTGRES_INVALID_BINARY_DATA/ }` alongside the `float8` case so both siblings are covered.
Comment thread
robobun marked this conversation as resolved.
{
name: "timestamp with 4-byte datum",
oid: TIMESTAMP,
col: i32(0),
code: /ERR_POSTGRES_INVALID_BINARY_DATA/,
},
{
name: "bool with 0-byte datum",
oid: BOOL,
col: Buffer.alloc(0),
code: /ERR_POSTGRES_INVALID_BINARY_DATA/,
},
{
name: "bool with out-of-range value 2",
oid: BOOL,
col: Buffer.from([2]),
code: /ERR_POSTGRES_INVALID_BINARY_DATA/,
},
{
// 20-byte header declares one element, but its length prefix claims 8
// bytes for a 4-byte int4 element.
name: "int4[] element length prefix != 4",
oid: INT4_ARRAY,
col: Buffer.concat([i32(1), i32(0), i32(INT4), i32(1), i32(1), i32(8), i64(0n)]),
code: /ERR_POSTGRES_INVALID_BINARY_DATA/,
},
{
name: "time = 2^63-1 microseconds (out of range)",
oid: TIME,
col: i64(0x7fffffffffffffffn),
code: /ERR_POSTGRES_INVALID_TIME_FORMAT/,
},
{
name: "time = -1 microseconds (negative)",
oid: TIME,
col: i64(-1n),
code: /ERR_POSTGRES_INVALID_TIME_FORMAT/,
},
{
// ndigits=0 header followed by trailing junk bytes.
name: "numeric with trailing bytes after ndigits=0",
oid: NUMERIC,
col: Buffer.concat([numericHeader(0, 0, 0x0000, 0), Buffer.from([0xde, 0xad, 0xbe, 0xef])]),
code: /ERR_POSTGRES_UNSUPPORTED_NUMERIC_FORMAT/,
},
{
// dscale read as a signed int16 is negative (0x8000 = -32768).
name: "numeric with negative dscale",
oid: NUMERIC,
col: Buffer.concat([numericHeader(1, 0, 0x0000, 0x8000), i16(1)]),
code: /ERR_POSTGRES_UNSUPPORTED_NUMERIC_FORMAT/,
},
{
// 16 raw bytes with a binary format code on a type Bun cannot binary-decode.
name: "uuid sent with binary format code",
oid: UUID,
col: Buffer.alloc(16, 0xab),
code: /ERR_POSTGRES_UNKNOWN_FORMAT_CODE/,
},
];

test.concurrent.each(malformed)("binary $name is rejected", async ({ oid, col, code }) => {
let err: any;
try {
await runMockQuery(col, oid);
} catch (e) {
err = e;
}
expect(err).toBeDefined();
expect(err?.code ?? err?.message).toMatch(code);
});

test.concurrent("well-formed binary bool still parses", async () => {
const result: any = await runMockQuery(Buffer.from([1]), BOOL);
expect(result[0].c).toBe(true);
});

test.concurrent("well-formed binary float8 still parses", async () => {
const buf = Buffer.alloc(8);
buf.writeDoubleBE(1.5, 0);
const result: any = await runMockQuery(buf, FLOAT8);
expect(result[0].c).toBe(1.5);
});

test.concurrent("in-range binary time still parses", async () => {
// 01:02:03 = 3723 seconds = 3_723_000_000 microseconds.
const result: any = await runMockQuery(i64(3_723_000_000n), TIME);
expect(result[0].c).toBe("01:02:03");
});

test.concurrent("well-formed binary numeric still parses", async () => {
const col = Buffer.concat([numericHeader(1, 0, 0x0000, 0), i16(1)]);
const result: any = await runMockQuery(col, NUMERIC);
expect(result[0].c).toBe("1");
});
Loading