Postgres log readers: the planted-input paths the #4051 review left open

Follow-up from the round-1 security review of #4051 (#4046 part 1). Each item needs a live plant or a design call, so none of them went into that PR.

The deadlock and plan-capture regexes can be fed about 64 MB per call on the target (review L2). The binary route runs encode(bytea, 'escape') before the server-side regex, and escape() turns each byte at or above 0x80 into four characters. A client that plants enough high bytes through failed logins can grow a 4 MB tail into 16 MB of text, which the reviewer puts at about 64 MB inside the regex engine. A cheap bytea position() guard on each collector's marker text would skip the escape and the regex on most cycles.
An RDS or Aurora fetch that meets a planted byte might leave the resume marker stuck. Untested. It needs a planted byte on a live RDS or Aurora instance.
A forged plan-capture line can blind plan capture on both routes. The server-side ::bigint and ::double precision casts of regex captures fail the whole read when a forged number overflows them.
Encodings other than UTF8 and LATIN1. Byte route (pg_read_binary_file) and managed-zone log_timezone (#4046 parts 1b, 1c) #4051 keeps every database that is not UTF8 on the text route, because the binary route decodes as UTF-8. A multibyte encoding such as EUC_JP can still fail validation (22021) on a planted byte. A single-byte encoding with undefined code points, such as WIN1252, may fail the conversion to the client encoding instead (22P05). Neither is tested.

Review: #4051 (comment)

#4058 L1/L2/L4: skip+count forged plan captures, splice marker litera…

#4058 L3/M2 doc fix: unit-pin both binary-route guards, fix stale pg_…

Decision on item 1's bound (maintainer delegated it to the client-site coordinator, 2026-09-24): accept the bounded cost. Neither cap ships.

The round-1 review of #4089 found that the binary route's position() guard is an optimization, not a bound. An attacker can plant the marker, so a high-byte-stuffed tail still costs about 4× the text route's regex work. It found two ways to bound it:

A cap on the escaped length, skipping the regex past it. This turns a CPU cost into blinding: stuffing high bytes would then hide every real deadlock and plan in the tail, which is the outcome A client can spell a pid and label into %u/%d and refuse or forge a target's log read, under any prefix with %u/%d before %p (the RDS default) #4046 and Byte route (pg_read_binary_file) and managed-zone log_timezone (#4046 parts 1b, 1c) #4051 exist to prevent. Blinding is worse than a constant-factor cost.
A quarter-size binary tail. This costs every binary-route target three quarters of its coverage on every cycle, to shave an attack that needs tens of thousands of failed logins to sustain.

So neither ships. The remaining exposure is bounded:

a constant factor tied to the tail size, not runaway;
per collector, per cycle, in the monitoring session's own backend;
loud: sustaining it takes a failed-login flood, and pg_log_events already collects those FATAL authentication lines, so the attack shows up in Darling's own data.

Reopen this if a real target shows sustained high-byte stuffing in its log, or the binary-route read shows up as a measurable CPU cost.

With that, item 1 is decided and item 3 ships in #4089. Items 2 and 4 stay open on this issue.

Log readers: a forged number can't blind plan capture, and the binary…

PR #4089 merged to dev as 976e091 (2026-09-24 01:39Z). It delivers items 1 and 3:

A forged plan-capture log line with an out-of-range query id or duration is now skipped and counted (forged_captures_skipped), instead of failing the whole read.
On the pg_read_binary_file route, the deadlock and plan-capture readers skip the escape-and-regex pass when the tail holds nothing they could match.

Item 1's remaining cost was accepted as bounded; see the decision comment above. This issue stays open for items 2 and 4.

Decode pg_read_binary_file log tail in the connected database's serve…

PR #4106 merged to dev as deacddd (2026-09-24 03:01Z). It delivers item 4: the pg_read_binary_file log route now decodes the tail in the database's own server_encoding instead of assuming UTF-8. It splits lines on the newline byte before decoding, so a planted multi-byte lead byte can't swallow the next line, and an undecodable byte becomes U+FFFD instead of failing the read. Encodings with no .NET mapping stay gated, as documented in #4062.

This issue stays open for item 2, the only one left.

A new item for this issue, found in the #4136 and #4137 review (deferred here, not a new issue): records that imitate a deadlock report or an auto_explain capture, raised by a client as genuine log records.

The csvlog parser (#4104, #4124 and #4140) keeps text planted inside quoted fields from becoming records. A client can still write a genuine record whose content imitates a deadlock report or a plan capture:

For deadlocks: a procedural-language RAISE at ERROR severity with a chosen message, detail and error code.
For plan capture: a RAISE whose message starts with the duration-plan marker. 4053 part b2: pg_plan_capture reads csvlog when it's on (self-hosted) #4137 now requires LOG severity, as the stderr regex does, but a LOG-level RAISE remains.

The stderr route has always been exposed the same way. Neither PR widens it.

Why it isn't closed in those PRs: no cheap discriminator covers every case.

sql_state_code can be set by the RAISE.
The csvlog context column's innermost frame names a PL/pgSQL RAISE. But PL/Perl and PL/Tcl add frames of their own, errhidecontext suppresses the frame, and a C function can raise with no frame at all.
The location column would name the lock manager's own reporting function, but only under log_error_verbosity = verbose.

A check that covers only PL/pgSQL would suggest the hole is closed when it isn't.

Options for whoever takes this:

Accept it and document it: a client can forge a report about its own session, and nothing else.
Use the context frame as a partial filter, and label it as partial.
Read the location column when verbose error logging is on.

Ruling on the RAISE-shaped-records item (the coordinator's call): a partial filter, labelled partial, plus a definitive check when verbose logging is on. A lane is on it.

The partial filter. A deadlock or plan record whose innermost context frame is a PL/pgSQL RAISE is skipped and counted. A genuine deadlock or auto_explain record raised while a function runs names the statement it was on, never the RAISE. The lane proves that live with a real deadlock inside a function. PL/Perl, PL/Tcl and C functions, and errhidecontext, are not caught, so the code says the filter is partial.
The definitive check. When log_error_verbosity = verbose, the csvlog location column and the jsonlog func_name name the reporting function, and a record naming the wrong one is skipped.
Not covered: the self-hosted stderr route, whose deadlocks come from a server-side regex rather than a parsed record.
Item 2 (a live RDS/Aurora planted-byte test) still needs a throwaway instance, which is an AWS-cost call for the maintainer.

4058 review round 1: seam list, provenance-guard order, RaisedByPlpgs…

RAISE-shaped deadlock and plan-capture records are not stored (#4058) (…

PR #4153 merged to dev as e7014aa (2026-09-24 11:19Z). Deadlock and plan-capture records that a PL/pgSQL function forged with RAISE are no longer stored. On the csvlog and jsonlog routes, a record whose context shows it came from a RAISE statement is counted and skipped, for self-hosted targets and on the RDS deadlock route. A real deadlock that happens inside a PL/pgSQL function is still kept. This issue stays open for item 2, the live RDS test.

The RAISE-shaped-records item has shipped in #4153 (merged as e7014aa).

A deadlock or plan-capture record whose innermost context frame is a PL/pgSQL RAISE is skipped and counted. The check is a quote-aware frame parse, so a client-chosen function name can't defeat it.
Under verbose error logging, so is a record whose reporting function isn't PostgreSQL's own (DeadLockReport for deadlocks, explain_ExecutorEnd for plans).
Covered: the csvlog routes (self-hosted and RDS, counted), and both stderr deadlock routes, silently.
Not covered: the stderr plan routes, servers at terse verbosity, and a non-English lc_messages. The code says so.

Still open here: item 2, the RDS/Aurora planted-byte resume-marker test. It needs a live RDS or Aurora instance to plant a byte through, and that is a cost decision for the maintainer.

Separately, #4160 tracks that CI skips the csvlog, jsonlog and auto_explain live classes, this item's live test included, because their environment variables are unset. A lane is fixing it.

Ruling (the maintainer, 2026-09-24): item 2 is an accepted risk. No live RDS or Aurora test will be run. This was the last open item, so the issue closes.

The risk: a planted byte might make every DownloadDBLogFilePortion fetch of one log portion fail, or make the store step fail on it. The resume marker would then never advance, and the reader would retry the same portion each cycle.

Why it is bounded (read from dev's source, not tested live):

The marker is per file. RdsLogSource.ReadNewestAsync keys it as instance plus file name, and re-lists the newest log file on every cycle. When the log rotates, the key changes. The stuck marker is left behind, and the reader starts on the new file: from its start on csvlog, or from the bounded tail on stderr.
The worst case is losing the rest of one log file's events for that target. That is up to one rotation interval: 60 minutes by default on RDS for PostgreSQL. The fleet's own setting was not checked.
The stall is not silent. A failed fetch is rethrown as RdsLogUnavailableException (RdsLogEventIngestor.cs, the catch around ReadNewestAsync), so the run is recorded as failed, not as a success with zero rows. The marker moves only after a successful store (CommitResume), which is the RdsLogSource advances the resume marker inside the fetch, so any failure before the write loses that chunk permanently #3008 rule.
The deadlock and plan readers on RDS go through the same ReadNewestAsync and share the same bound.

Reopen if collection health shows a target's RDS log reads failing across a rotation boundary, meaning the stall outlives one file.

Navigation Menu

Repository navigation

Postgres log readers: the planted-input paths the #4051 review left open #4058

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Postgres log readers: the planted-input paths the #4051 review left open #4058

Description

Activity

erikdarlingdata commented on Sep 24, 2026

erikdarlingdata commented on Sep 24, 2026

erikdarlingdata commented on Sep 24, 2026

erikdarlingdata commented on Sep 24, 2026

erikdarlingdata commented on Sep 24, 2026

erikdarlingdata commented on Sep 24, 2026

erikdarlingdata commented on Sep 24, 2026

erikdarlingdata commented on Sep 24, 2026

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions