From e92c67ddc09727a8b7395041fe0e57cd2f595f79 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 16 Dec 2014 15:35:43 -0500 Subject: [PATCH] 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. --- src/backend/bootstrap/bootscanner.l | 2 +- src/backend/bootstrap/bootstrap.c | 31 ++++++++++++----------------- src/include/bootstrap/bootstrap.h | 2 +- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/backend/bootstrap/bootscanner.l b/src/backend/bootstrap/bootscanner.l index c308f85777..d3e41d564d 100644 --- a/src/backend/bootstrap/bootscanner.l +++ b/src/backend/bootstrap/bootscanner.l @@ -105,7 +105,7 @@ insert { return(INSERT_TUPLE); } "toast" { return(XTOAST); } {arrayid} { - yylval.str = pstrdup(MapArrayTypeName(yytext)); + yylval.str = MapArrayTypeName(yytext); return(ID); } {id} { diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index 9339771d58..3b4398b80b 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -1026,38 +1026,33 @@ AllocateAttribute(void) return attribute; } -/* ---------------- +/* * MapArrayTypeName - * XXX arrays of "basetype" are always "_basetype". - * this is an evil hack inherited from rel. 3.1. - * XXX array dimension is thrown away because we - * don't support fixed-dimension arrays. again, - * sickness from 3.1. * - * the string passed in must have a '[' character in it + * Given a type name, produce the corresponding array type name by prepending + * '_' and truncating as needed to fit in NAMEDATALEN-1 bytes. This is only + * used in bootstrap mode, so we can get away with assuming that the input is + * ASCII and we don't need multibyte-aware truncation. * - * the string returned is a pointer to static storage and should NOT - * be freed by the CALLER. - * ---------------- + * The given string normally ends with '[]' or '[digits]'; we discard that. + * + * The result is a palloc'd string. */ char * -MapArrayTypeName(char *s) +MapArrayTypeName(const char *s) { int i, j; - static char newStr[NAMEDATALEN]; /* array type names < NAMEDATALEN long */ + char newStr[NAMEDATALEN]; - if (s == NULL || s[0] == '\0') - return s; - - j = 1; newStr[0] = '_'; - for (i = 0; i < NAMEDATALEN - 1 && s[i] != '['; i++, j++) + j = 1; + for (i = 0; i < NAMEDATALEN - 2 && s[i] != '['; i++, j++) newStr[j] = s[i]; newStr[j] = '\0'; - return newStr; + return pstrdup(newStr); } diff --git a/src/include/bootstrap/bootstrap.h b/src/include/bootstrap/bootstrap.h index 9fe2e0f3f2..173745d783 100644 --- a/src/include/bootstrap/bootstrap.h +++ b/src/include/bootstrap/bootstrap.h @@ -40,7 +40,7 @@ extern void InsertOneTuple(Oid objectid); extern void InsertOneValue(char *value, int i); extern void InsertOneNull(int i); -extern char *MapArrayTypeName(char *s); +extern char *MapArrayTypeName(const char *s); extern void index_register(Oid heap, Oid ind, IndexInfo *indexInfo); extern void build_indices(void); -- 2.40.0