Skip to content

Plan capture reads a plan block anywhere in a log line, so a statement's author can attach a forged plan to any query id #4008

Closed

Description

@erikdarlingdata

Found by #3996's round-2 security review. It predates #3996 and is spoofing, not a leak of another session's data.

What is wrong

Plan capture finds auto_explain's report with a pattern that is not anchored to the start of a log line:

  • PgPlanLogParser.s_planBlock: \[\d+\][ :](-?\d+) LOG: duration: ([0-9.]+) ms plan:\s*\n((?:\t[^\n]*\n)+)
  • the self-hosted route's regexp_matches in PgPlanCaptureCollector.QueryText (the same pattern in SQL).

So text anywhere in a line can open a plan block. A statement's author can write the header into their own SQL, and the log echoes it back in the STATEMENT: companion. The tab-led lines under that echo are then read as the plan:

... ERROR:  syntax error at or near "{" at character 60
... STATEMENT:  SELECT 1 -- [1] 7 LOG:  duration: 1.0 ms  plan:
	{"Plan": {"Node Type": "Seq Scan", "Relation Name": "HUNTER2_LEAK", "Alias": "card 4111111111111111"}}

PgPlanLogParser.Extract returns one plan here: query id 7, duration 1 ms, with the forged JSON. The author chooses the query id, so the forged plan attaches to any real query's history, with any duration.

Impact

  • A user who can run SQL on a monitored target can plant plans, and the durations that come with them, on queries they do not own. Plan history, regressions and advice built on it can be steered.
  • The forged plan is the author's own text. It is not another session's data, so this is not a confidentiality leak.

Suggested fix (the reviewer's)

Extract plans from ASSEMBLED entries (PgLogEntryAssembler), not from raw text:

  • take only an entry whose severity is LOG and whose message head is auto_explain's duration: N ms plan:;
  • take the query id from the entry's prefix (%Q), not from text inside the line.

A companion line (STATEMENT:, QUERY:) can then never open a plan. The self-hosted route's SQL-side regexp_matches needs the same change, or the parse should move out of SQL.

Lite parity: check whether Lite's PostgreSQL plan capture shares the pattern.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions