Skip to content

Commit 65da0d6

Browse files
tglsfdctglsfdc
committed
Fix misuse of StrNCpy to copy and add null to non-null-terminated data.
Does not work since it fetches one byte beyond the source data, and when the phase of the moon is wrong, the source data is smack up against the end of backend memory and you get SIGSEGV. Don't laugh, this is a fix for an actual user bug report.
1 parent de85dd1 commit 65da0d6

File tree

9 files changed

+56
-43
lines changed

9 files changed

+56
-43
lines changed

‎contrib/miscutil/misc_utils.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,9 @@ active_listeners(text *relname)
9999

100100
if (relname && (VARSIZE(relname) > VARHDRSZ))
101101
{
102+
MemSet(listen_name, 0, NAMEDATALEN);
102103
len = MIN(VARSIZE(relname) - VARHDRSZ, NAMEDATALEN - 1);
103-
strncpy(listen_name, VARDATA(relname), len);
104-
listen_name[len] = '\0';
104+
memcpy(listen_name, VARDATA(relname), len);
105105
ScanKeyEntryInitialize(&key, 0,
106106
Anum_pg_listener_relname,
107107
F_NAMEEQ,

‎src/backend/libpq/be-fsstubs.c

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/libpq/be-fsstubs.c,v 1.48 2000/07/03 23:09:37 wieck Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/libpq/be-fsstubs.c,v 1.49 2000/07/07 21:12:53 tgl Exp $
1212
*
1313
* NOTES
1414
* This should be moved to a more appropriate place. It is here
@@ -379,26 +379,23 @@ lo_import(PG_FUNCTION_ARGS)
379379
/*
380380
* open the file to be read in
381381
*/
382-
nbytes = VARSIZE(filename) - VARHDRSZ + 1;
383-
if (nbytes > FNAME_BUFSIZE)
384-
nbytes = FNAME_BUFSIZE;
385-
StrNCpy(fnamebuf, VARDATA(filename), nbytes);
382+
nbytes = VARSIZE(filename) - VARHDRSZ;
383+
if (nbytes >= FNAME_BUFSIZE)
384+
nbytes = FNAME_BUFSIZE-1;
385+
memcpy(fnamebuf, VARDATA(filename), nbytes);
386+
fnamebuf[nbytes] = '\0';
386387
fd = PathNameOpenFile(fnamebuf, O_RDONLY | PG_BINARY, 0666);
387388
if (fd < 0)
388-
{ /* error */
389389
elog(ERROR, "lo_import: can't open unix file \"%s\": %m",
390390
fnamebuf);
391-
}
392391

393392
/*
394393
* create an inversion "object"
395394
*/
396395
lobj = inv_create(INV_READ | INV_WRITE);
397396
if (lobj == NULL)
398-
{
399397
elog(ERROR, "lo_import: can't create inv object for \"%s\"",
400398
fnamebuf);
401-
}
402399

403400
/*
404401
* the oid for the large object is just the oid of the relation
@@ -461,18 +458,17 @@ lo_export(PG_FUNCTION_ARGS)
461458
* 022. This code used to drop it all the way to 0, but creating
462459
* world-writable export files doesn't seem wise.
463460
*/
464-
nbytes = VARSIZE(filename) - VARHDRSZ + 1;
465-
if (nbytes > FNAME_BUFSIZE)
466-
nbytes = FNAME_BUFSIZE;
467-
StrNCpy(fnamebuf, VARDATA(filename), nbytes);
461+
nbytes = VARSIZE(filename) - VARHDRSZ;
462+
if (nbytes >= FNAME_BUFSIZE)
463+
nbytes = FNAME_BUFSIZE-1;
464+
memcpy(fnamebuf, VARDATA(filename), nbytes);
465+
fnamebuf[nbytes] = '\0';
468466
oumask = umask((mode_t) 0022);
469467
fd = PathNameOpenFile(fnamebuf, O_CREAT | O_WRONLY | O_TRUNC | PG_BINARY, 0666);
470468
umask(oumask);
471469
if (fd < 0)
472-
{ /* error */
473470
elog(ERROR, "lo_export: can't open unix file \"%s\": %m",
474471
fnamebuf);
475-
}
476472

477473
/*
478474
* read in from the Unix file and write to the inversion file

‎src/backend/libpq/be-pqexec.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $Header: /cvsroot/pgsql/src/backend/libpq/Attic/be-pqexec.c,v 1.35 2000/07/05 23:11:19 tgl Exp $
12+
* $Header: /cvsroot/pgsql/src/backend/libpq/Attic/be-pqexec.c,v 1.36 2000/07/07 21:12:53 tgl Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -236,8 +236,8 @@ strmake(char *str, int len)
236236
if (len <= 0)
237237
len = strlen(str);
238238

239-
newstr = (char *) palloc((unsigned) len + 1);
240-
StrNCpy(newstr, str, len + 1);
239+
newstr = (char *) palloc(len + 1);
240+
memcpy(newstr, str, len);
241241
newstr[len] = (char) 0;
242242
return newstr;
243243
}

‎src/backend/port/dynloader/aix.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,8 @@ readExports(ModulePtr mp)
536536
* first SYMNMLEN chars and make sure we have a zero byte at
537537
* the end.
538538
*/
539-
StrNCpy(tmpsym, ls->l_name, SYMNMLEN + 1);
539+
strncpy(tmpsym, ls->l_name, SYMNMLEN);
540+
tmpsym[SYMNMLEN] = '\0';
540541
symname = tmpsym;
541542
}
542543
ep->name = strdup(symname);

‎src/backend/utils/adt/like.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
* Portions Copyright (c) 1994, Regents of the University of California
1212
*
1313
* IDENTIFICATION
14-
* $Header: /cvsroot/pgsql/src/backend/utils/adt/like.c,v 1.36 2000/07/06 05:48:11 tgl Exp $
14+
* $Header: /cvsroot/pgsql/src/backend/utils/adt/like.c,v 1.37 2000/07/07 21:12:50 tgl Exp $
1515
*
1616
*-------------------------------------------------------------------------
1717
*/
@@ -48,7 +48,8 @@ fixedlen_like(char *s, text *p, int charlen)
4848
(void) pg_mb2wchar_with_len((unsigned char *) s, sterm, charlen);
4949
#else
5050
sterm = (char *) palloc(charlen + 1);
51-
StrNCpy(sterm, s, charlen + 1);
51+
memcpy(sterm, s, charlen);
52+
sterm[charlen] = '\0';
5253
#endif
5354

5455
/*

‎src/backend/utils/adt/not_in.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/utils/adt/Attic/not_in.c,v 1.23 2000/06/09 01:11:09 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/utils/adt/Attic/not_in.c,v 1.24 2000/07/07 21:12:50 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -52,10 +52,12 @@ int4notin(PG_FUNCTION_ARGS)
5252
char my_copy[NAMEDATALEN * 2 + 2];
5353
Datum value;
5454

55-
strlength = VARSIZE(relation_and_attr) - VARHDRSZ + 1;
56-
if (strlength > sizeof(my_copy))
57-
strlength = sizeof(my_copy);
58-
StrNCpy(my_copy, VARDATA(relation_and_attr), strlength);
55+
/* make a null-terminated copy of text */
56+
strlength = VARSIZE(relation_and_attr) - VARHDRSZ;
57+
if (strlength >= sizeof(my_copy))
58+
strlength = sizeof(my_copy)-1;
59+
memcpy(my_copy, VARDATA(relation_and_attr), strlength);
60+
my_copy[strlength] = '\0';
5961

6062
relation = (char *) strtok(my_copy, ".");
6163
attribute = (char *) strtok(NULL, ".");

‎src/backend/utils/adt/regexp.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/utils/adt/regexp.c,v 1.32 2000/07/06 05:48:11 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/utils/adt/regexp.c,v 1.33 2000/07/07 21:12:50 tgl Exp $
1212
*
1313
* Alistair Crooks added the code for the regex caching
1414
* agc - cached the regular expressions used - there's a good chance
@@ -164,7 +164,8 @@ fixedlen_regexeq(char *s, text *p, int charlen, int cflags)
164164

165165
/* be sure sterm is null-terminated */
166166
sterm = (char *) palloc(charlen + 1);
167-
StrNCpy(sterm, s, charlen + 1);
167+
memcpy(sterm, s, charlen);
168+
sterm[charlen] = '\0';
168169

169170
result = RE_compile_and_execute(p, sterm, cflags);
170171

‎src/backend/utils/adt/varchar.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/utils/adt/varchar.c,v 1.67 2000/07/03 23:09:53 wieck Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/utils/adt/varchar.c,v 1.68 2000/07/07 21:12:50 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -115,9 +115,11 @@ bpcharout(PG_FUNCTION_ARGS)
115115
char *result;
116116
int len;
117117

118+
/* copy and add null term */
118119
len = VARSIZE(s) - VARHDRSZ;
119120
result = (char *) palloc(len + 1);
120-
StrNCpy(result, VARDATA(s), len + 1); /* copy and add null term */
121+
memcpy(result, VARDATA(s), len);
122+
result[len] = '\0';
121123

122124
#ifdef CYR_RECODE
123125
convertstr(result, len, 1);
@@ -268,8 +270,8 @@ bpchar_name(char *s)
268270
return NULL;
269271

270272
len = VARSIZE(s) - VARHDRSZ;
271-
if (len > NAMEDATALEN)
272-
len = NAMEDATALEN;
273+
if (len >= NAMEDATALEN)
274+
len = NAMEDATALEN-1;
273275

274276
while (len > 0)
275277
{
@@ -284,7 +286,7 @@ bpchar_name(char *s)
284286
#endif
285287

286288
result = (NameData *) palloc(NAMEDATALEN);
287-
StrNCpy(NameStr(*result), VARDATA(s), NAMEDATALEN);
289+
memcpy(NameStr(*result), VARDATA(s), len);
288290

289291
/* now null pad to full length... */
290292
while (len < NAMEDATALEN)
@@ -316,7 +318,7 @@ name_bpchar(NameData *s)
316318
#endif
317319

318320
result = (char *) palloc(VARHDRSZ + len);
319-
strncpy(VARDATA(result), NameStr(*s), len);
321+
memcpy(VARDATA(result), NameStr(*s), len);
320322
VARATT_SIZEP(result) = len + VARHDRSZ;
321323

322324
return result;
@@ -365,9 +367,11 @@ varcharout(PG_FUNCTION_ARGS)
365367
char *result;
366368
int len;
367369

370+
/* copy and add null term */
368371
len = VARSIZE(s) - VARHDRSZ;
369372
result = (char *) palloc(len + 1);
370-
StrNCpy(result, VARDATA(s), len + 1); /* copy and add null term */
373+
memcpy(result, VARDATA(s), len);
374+
result[len] = '\0';
371375

372376
#ifdef CYR_RECODE
373377
convertstr(result, len, 1);

‎src/include/c.h

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1996-2000, PostgreSQL, Inc
99
* Portions Copyright (c) 1994, Regents of the University of California
1010
*
11-
* $Id: c.h,v 1.75 2000/07/06 21:33:44 petere Exp $
11+
* $Id: c.h,v 1.76 2000/07/07 21:12:47 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -781,11 +781,19 @@ extern int assertTest(int val);
781781

782782
/*
783783
* StrNCpy
784-
* Like standard library function strncpy(), except that result string
785-
* is guaranteed to be null-terminated --- that is, at most N-1 bytes
786-
* of the source string will be kept.
787-
* Also, the macro returns no result (too hard to do that without
788-
* evaluating the arguments multiple times, which seems worse).
784+
* Like standard library function strncpy(), except that result string
785+
* is guaranteed to be null-terminated --- that is, at most N-1 bytes
786+
* of the source string will be kept.
787+
* Also, the macro returns no result (too hard to do that without
788+
* evaluating the arguments multiple times, which seems worse).
789+
*
790+
* BTW: when you need to copy a non-null-terminated string (like a text
791+
* datum) and add a null, do not do it with StrNCpy(..., len+1). That
792+
* might seem to work, but it fetches one byte more than there is in the
793+
* text object. One fine day you'll have a SIGSEGV because there isn't
794+
* another byte before the end of memory. Don't laugh, we've had real
795+
* live bug reports from real live users over exactly this mistake.
796+
* Do it honestly with "memcpy(dst,src,len); dst[len] = '\0';", instead.
789797
*/
790798
#define StrNCpy(dst,src,len) \
791799
do \

0 commit comments

Comments
 (0)