From: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
---|---|
To: | ranier(dot)vf(at)gmail(dot)com |
Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Explicit NULL dereference (src/backend/utils/adt/ruleutils.c) |
Date: | 2020-11-02 02:19:09 |
Message-ID: | 20201102.111909.804013494781872450.horikyota.ntt@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
At Mon, 02 Nov 2020 10:36:10 +0900 (JST), Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote in
> At Sat, 31 Oct 2020 11:49:07 -0300, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote in
> > Per Coverity.
> >
> > make_ruledef function can dereference a NULL pointer (actions),
> > if "ev_qual" is provided and "actions" does not exist.
> >
> > The comment there is contradictory: " /* these could be nulls */ "
> > Because if "ev_qual" is not null, "actions" cannot be either.
> >
> > Solution proposed merely as a learning experience.
>
> We cannot reach there with ev_action == NULL since it comes from a
> non-nullable column. Since most of the other columns has an assertion
> that !isnull, I think we should do the same thing for ev_action (and
> ev_qual). SPI_getvalue() returns C-NULL for SQL-NULL (or for some
> other unexpected situations.).
The following code is found there, since 1998. (15cb32d93e)
> /* If the rule has an event qualification, add it */
> if (ev_qual == NULL)
> ev_qual = "";
The problem code here was written as the follows.
+ fno = SPI_fnumber(rulettc, "is_instead");
+ is_instead = (bool)SPI_getbinval(ruletup, rulettc, fno, &isnull);
+
+ fno = SPI_fnumber(rulettc, "ev_qual");
+ ev_qual = SPI_getvalue(ruletup, rulettc, fno);
+ if (isnull) ev_qual = NULL;
+
+ fno = SPI_fnumber(rulettc, "ev_action");
+ ev_action = SPI_getvalue(ruletup, rulettc, fno);
+ if (isnull) ev_action = NULL;
+ if (ev_action != NULL) {
+ actions = (List *)stringToNode(ev_action);
+ }
I'm not sure what the code means by just reading there but at least it
seems impossible for the current code to return NULL for legit values.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachment | Content-Type | Size |
---|---|---|
ev_qual_and_action_catnnot_be_null_2.patch | text/x-patch | 1.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Nikhil Benesch | 2020-11-02 02:39:11 | [PATCH] Support negative indexes in split_part |
Previous Message | Tom Lane | 2020-11-02 02:05:29 | Re: Explicit NULL dereference (src/backend/utils/adt/ruleutils.c) |