Subject: Fix out-of-bounds in the function GetCommandTagName

From: Ranier Vilela <[email-masked]>
Date: Sun, 14 Apr 2024 20:17:35 -0300

Hi,

Per Coverity.

Coverity has reported some out-of-bounds bugs
related to the GetCommandTagName function.

CID 1542964: (#1 of 1): Out-of-bounds access (OVERRUN)
7. overrun-call: Overrunning callee's array of size 193 by passing argument
commandtag (which evaluates to 193) in call to GetCommandTagName.[

It turns out that the root of the problem is found in the declaration of
the tag_behavior array, which is found in src/backend/tcop/cmdtag.c.

The size of the array is defined by COMMAND_TAG_NEXTTAG enum,
whose value currently corresponds to 193.
Since enum items are evaluated starting at zero, by default.

It turns out that the final size of the array, 193, limits the number of
items to 192, which excludes the last TAG
PG_CMDTAG(CMDTAG_VACUUM, "VACUUM", false, false, false)

Fixed leaving it up to the compiler to determine the final size of the
array.

Patch attached.

best regards,
Ranier Vilela

---

From: David Rowley <[email-masked]>
Date: Mon, 15 Apr 2024 11:38:09 +1200

> Since enum items are evaluated starting at zero, by default.

I think the change makes sense. I don't see any good reason to define
COMMAND_TAG_NEXTTAG or force the compiler's hand when it comes to
sizing that array.

Clearly, Coverity does not understand that we'll never call any of
those GetCommandTag* functions with COMMAND_TAG_NEXTTAG.

> Patch attached.

You seem to have forgotten to attach it, but my comments above were
written with the assumption that the patch is what I've attached here.

David
diff --git a/src/backend/tcop/cmdtag.c b/src/backend/tcop/cmdtag.c
index 68689b3e0d..0870064fdd 100644
--- a/src/backend/tcop/cmdtag.c
+++ b/src/backend/tcop/cmdtag.c
@@ -30,7 +30,7 @@ typedef struct CommandTagBehavior
 #define PG_CMDTAG(tag, name, evtrgok, rwrok, rowcnt) \
 	{ name, (uint8) (sizeof(name) - 1), evtrgok, rwrok, rowcnt },
 
-static const CommandTagBehavior tag_behavior[COMMAND_TAG_NEXTTAG] = {
+static const CommandTagBehavior tag_behavior[] = {
 #include "tcop/cmdtaglist.h"
 };
 
diff --git a/src/include/tcop/cmdtag.h b/src/include/tcop/cmdtag.h
index da210e54fa..23c99d7eca 100644
--- a/src/include/tcop/cmdtag.h
+++ b/src/include/tcop/cmdtag.h
@@ -22,7 +22,6 @@
 typedef enum CommandTag
 {
 #include "tcop/cmdtaglist.h"
-	COMMAND_TAG_NEXTTAG
 } CommandTag;
 
 #undef PG_CMDTAG

---

From: Ranier Vilela <[email-masked]>
Date: Sun, 14 Apr 2024 21:12:31 -0300

> those GetCommandTag* functions with COMMAND_TAG_NEXTTAG.
I think that Coverity understood it this way because when
including COMMAND_TAG_NEXTTAG, in the enum definition,
led to 193 items, and the last item in the array is currently 192.

> written with the assumption that the patch is what I've attached here.
Yes, I actually forgot.

+1 for your patch.

best regards,
Ranier Vilela