Skip to content

Commit 689959e

Browse files
committed
Silece stringop-overflow warning and protect against invalid read
strncat unfortunately takes a misleading size argument which means at most size from src. It is a bit of an antipattern in the string family of functions and for that compilers will emit a warning if it happens that the size argument matches the size of src, since that is not what usually users of strncat want to do. The usage of the function in the code was correct. However instead of silencing the compiler, strncat was replaced with the dynamic buffer family of operations that postgres provides for the frontend. Also protect against an invalid read in case that the size of the result is zero. Reviewed-by: Asim R P <apraveen@pivotal.io> Reviewed-by: Daniel Gustafsson <dgustafsson@pivotal.io>
1 parent 6f3ed93 commit 689959e

1 file changed

Lines changed: 28 additions & 27 deletions

File tree

src/bin/pg_dump/dumputils.c

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1529,36 +1529,35 @@ escape_fmtopts_string(const char *src)
15291529
char *
15301530
custom_fmtopts_string(const char *src)
15311531
{
1532-
int len = src ? strlen(src) : 0;
1533-
char *result = calloc(1, len * 2 + 2);
1534-
char *srcdup = src ? strdup(src) : NULL;
1535-
char *srcdup_start = srcdup;
1536-
char *find_res = NULL;
1537-
int last = 0;
1538-
1539-
if (!srcdup || !result)
1540-
{
1541-
if (result)
1542-
free(result);
1543-
if (srcdup)
1544-
free(srcdup);
1532+
PQExpBufferData result;
1533+
char *srcdup;
1534+
char *to_free;
1535+
char *find_res;
1536+
char *srcdup_end;
1537+
int last;
1538+
1539+
if (!src)
15451540
return NULL;
1546-
}
1541+
1542+
to_free = srcdup = pg_strdup(src);
1543+
srcdup_end = srcdup + strlen(srcdup);
1544+
initPQExpBuffer(&result);
15471545

15481546
while (srcdup)
15491547
{
15501548
/* find first word (a) */
15511549
find_res = strchr(srcdup, ' ');
15521550
if (!find_res)
15531551
break;
1554-
strncat(result, srcdup, (find_res - srcdup));
1552+
*find_res = '\0';
1553+
appendPQExpBufferStr(&result, srcdup);
15551554
/* skip space */
15561555
srcdup = find_res + 1;
15571556
/* remove E if E' */
15581557
if ((strlen(srcdup) > 2) && (srcdup[0] == 'E') && (srcdup[1] == '\''))
15591558
srcdup++;
15601559
/* add " = " */
1561-
strncat(result, " = ", 3);
1560+
appendPQExpBuffer(&result, " = ");
15621561
/* find second word (b) until second '
15631562
find \' combinations and ignore them */
15641563
find_res = strchr(srcdup + 1, '\'');
@@ -1568,23 +1567,25 @@ custom_fmtopts_string(const char *src)
15681567
}
15691568
if (!find_res)
15701569
break;
1571-
strncat(result, srcdup, (find_res - srcdup + 1));
1572-
srcdup = find_res + 1;
1573-
/* skip space and add ',' */
1574-
if (srcdup && srcdup[0] == ' ')
1570+
find_res++;
1571+
*find_res = '\0';
1572+
appendPQExpBufferStr(&result, srcdup);
1573+
srcdup = find_res;
1574+
/* move to the next token if exists and add ',' */
1575+
if (find_res < srcdup_end - 1)
15751576
{
1576-
srcdup++;
1577-
strncat(result, ",", 1);
1577+
srcdup = find_res + 1;
1578+
appendPQExpBuffer(&result, ",");
15781579
}
15791580
}
15801581

15811582
/* fix string - remove trailing ',' or '=' */
1582-
last = strlen(result) - 1;
1583-
if (result[last] == ',' || result[last] == '=')
1584-
result[last] = '\0';
1583+
last = strlen(result.data) - 1;
1584+
if (last >= 0 && (result.data[last] == ',' || result.data[last] == '='))
1585+
result.data[last] = '\0';
15851586

1586-
free(srcdup_start);
1587-
return result;
1587+
pg_free(to_free);
1588+
return result.data;
15881589
}
15891590

15901591
/*

0 commit comments

Comments
 (0)