On Sun, 2026-08-09 at 22:15 -0400, Tom Lane wrote:
> 874 if (!OidIsValid(serverid))
> > > > CID 1699896: Null pointer dereferences
> > > > (FORWARD_NULL)
> > > > Passing null pointer "conninfo" to "cstring_to_text", which
> > > > dereferences it.
> 875 values[Anum_pg_subscription_subconninfo - 1]
> =
> 876 CStringGetTextDatum(conninfo);
>
> AFAICS, it's right: if stmt->servername is set while opts.connect is
> not, we'll arrive at this step with serverid filled in but conninfo
> still NULL. Even if there's some upstream reason why that
> combination
> can't occur, this is pretty fragile-looking code.
If serverid is filled, that branch won't be taken.
The code relies on the grammar setting either stmt->servername or stmt-
>conninfo, but not both. I agree that the control flow shouldn't rely
on that, and the code could be more clear anyway. Patch attached.
Regards,
Jeff Davis
From 1506505498b879b736aa2be718667c07b8e9cef3 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
Date: Mon, 10 Aug 2026 06:59:29 -0700
Subject: [PATCH] Clarify logic in CreateSubscription().
No bug found in previous code, but it unnecessarily relied on grammar
rules. Per complaint from Coverity.
Reported-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 19
---
src/backend/commands/subscriptioncmds.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 8e8db08bd93..9938f4b7fe3 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -871,11 +871,17 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
values[Anum_pg_subscription_subretentionactive - 1] =
BoolGetDatum(opts.retaindeadtuples);
values[Anum_pg_subscription_subserver - 1] = ObjectIdGetDatum(serverid);
- if (!OidIsValid(serverid))
+ if (stmt->conninfo)
+ {
+ Assert(stmt->conninfo == conninfo && !OidIsValid(serverid));
values[Anum_pg_subscription_subconninfo - 1] =
- CStringGetTextDatum(conninfo);
+ CStringGetTextDatum(stmt->conninfo);
+ }
else
+ {
+ Assert(OidIsValid(serverid));
nulls[Anum_pg_subscription_subconninfo - 1] = true;
+ }
if (opts.slot_name)
values[Anum_pg_subscription_subslotname - 1] =
DirectFunctionCall1(namein, CStringGetDatum(opts.slot_name));
--
2.43.0
