Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 6670913

Browse files
committed
Fix off-by-one loop count in MapArrayTypeName, and get rid of static array.
MapArrayTypeName would copy up to NAMEDATALEN-1 bytes of the base type name, which of course is wrong: after prepending '_' there is only room for NAMEDATALEN-2 bytes. Aside from being the wrong result, this case would lead to overrunning the statically allocated work buffer. This would be a security bug if the function were ever used outside bootstrap mode, but it isn't, at least not in any currently supported branches. Aside from fixing the off-by-one loop logic, this patch gets rid of the static work buffer by having MapArrayTypeName pstrdup its result; the sole caller was already doing that, so this just requires moving the pstrdup call. This saves a few bytes but mainly it makes the API a lot cleaner. Back-patch on the off chance that there is some third-party code using MapArrayTypeName with less-secure input. Pushing pstrdup into the function should not cause any serious problems for such hypothetical code; at worst there might be a short term memory leak. Per Coverity scanning.
1 parent de8e46f commit 6670913

File tree

3 files changed

+15
-20
lines changed

3 files changed

+15
-20
lines changed

src/backend/bootstrap/bootscanner.l

+1-1
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ insert { return(INSERT_TUPLE); }
111111
"toast" { return(XTOAST); }
112112

113113
{arrayid} {
114-
yylval.str = pstrdup(MapArrayTypeName(yytext));
114+
yylval.str = MapArrayTypeName(yytext);
115115
return(ID);
116116
}
117117
{id} {

src/backend/bootstrap/bootstrap.c

+13-18
Original file line numberDiff line numberDiff line change
@@ -1032,38 +1032,33 @@ AllocateAttribute(void)
10321032
return attribute;
10331033
}
10341034

1035-
/* ----------------
1035+
/*
10361036
* MapArrayTypeName
1037-
* XXX arrays of "basetype" are always "_basetype".
1038-
* this is an evil hack inherited from rel. 3.1.
1039-
* XXX array dimension is thrown away because we
1040-
* don't support fixed-dimension arrays. again,
1041-
* sickness from 3.1.
10421037
*
1043-
* the string passed in must have a '[' character in it
1038+
* Given a type name, produce the corresponding array type name by prepending
1039+
* '_' and truncating as needed to fit in NAMEDATALEN-1 bytes. This is only
1040+
* used in bootstrap mode, so we can get away with assuming that the input is
1041+
* ASCII and we don't need multibyte-aware truncation.
10441042
*
1045-
* the string returned is a pointer to static storage and should NOT
1046-
* be freed by the CALLER.
1047-
* ----------------
1043+
* The given string normally ends with '[]' or '[digits]'; we discard that.
1044+
*
1045+
* The result is a palloc'd string.
10481046
*/
10491047
char *
1050-
MapArrayTypeName(char *s)
1048+
MapArrayTypeName(const char *s)
10511049
{
10521050
int i,
10531051
j;
1054-
static char newStr[NAMEDATALEN]; /* array type names < NAMEDATALEN long */
1052+
char newStr[NAMEDATALEN];
10551053

1056-
if (s == NULL || s[0] == '\0')
1057-
return s;
1058-
1059-
j = 1;
10601054
newStr[0] = '_';
1061-
for (i = 0; i < NAMEDATALEN - 1 && s[i] != '['; i++, j++)
1055+
j = 1;
1056+
for (i = 0; i < NAMEDATALEN - 2 && s[i] != '['; i++, j++)
10621057
newStr[j] = s[i];
10631058

10641059
newStr[j] = '\0';
10651060

1066-
return newStr;
1061+
return pstrdup(newStr);
10671062
}
10681063

10691064

src/include/bootstrap/bootstrap.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ extern void InsertOneTuple(Oid objectid);
4040
extern void InsertOneValue(char *value, int i);
4141
extern void InsertOneNull(int i);
4242

43-
extern char *MapArrayTypeName(char *s);
43+
extern char *MapArrayTypeName(const char *s);
4444

4545
extern void index_register(Oid heap, Oid ind, IndexInfo *indexInfo);
4646
extern void build_indices(void);

0 commit comments

Comments
 (0)