Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc182c1e

Browse files
committed
Clean up assorted misuses of snprintf()'s result value.
Fix a small number of places that were testing the result of snprintf()but doing so incorrectly. The right test for buffer overrun, per C99,is "result >= bufsize" not "result > bufsize". Some places were alsochecking for failure with "result == -1", but the standard only saysthat a negative value is delivered on failure.(Note that this only makes these places correct if snprintf() deliversC99-compliant results. But at least now these places are consistentwith all the other places where we assume that.)Also, make psql_start_test() and isolation_start_test() check forbuffer overrun while constructing their shell commands. There seemslike a higher risk of overrun, with more severe consequences, herethan there is for the individual file paths that are made elsewherein the same functions, so this seemed like a worthwhile change.Also fix guc.c's do_serialize() to initialize errno = 0 beforecalling vsnprintf. In principle, this should be unnecessary becausevsnprintf should have set errno if it returns a failure indication ...but the other two places this coding pattern is cribbed from don'tassume that, so let's be consistent.These errors are all very old, so back-patch as appropriate. I thinkthat only the shell command overrun cases are even theoreticallyreachable in practice, but there's not much point in erroneous errorchecks.Discussion:https://postgr.es/m/17245.1534289329@sss.pgh.pa.us
1 parent54db0e5 commitc182c1e

File tree

8 files changed

+46
-20
lines changed

8 files changed

+46
-20
lines changed

‎src/backend/libpq/ip.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ getnameinfo_unix(const struct sockaddr_un * sa, int salen,
240240
char*service,intservicelen,
241241
intflags)
242242
{
243-
intret=-1;
243+
intret;
244244

245245
/* Invalid arguments. */
246246
if (sa==NULL||sa->sun_family!=AF_UNIX||
@@ -250,14 +250,14 @@ getnameinfo_unix(const struct sockaddr_un * sa, int salen,
250250
if (node)
251251
{
252252
ret=snprintf(node,nodelen,"%s","[local]");
253-
if (ret==-1||ret>nodelen)
253+
if (ret<0||ret >=nodelen)
254254
returnEAI_MEMORY;
255255
}
256256

257257
if (service)
258258
{
259259
ret=snprintf(service,servicelen,"%s",sa->sun_path);
260-
if (ret==-1||ret>servicelen)
260+
if (ret<0||ret >=servicelen)
261261
returnEAI_MEMORY;
262262
}
263263

‎src/backend/postmaster/pgstat.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4070,7 +4070,7 @@ get_dbstat_filename(bool permanent, bool tempname, Oid databaseid,
40704070
pgstat_stat_directory,
40714071
databaseid,
40724072
tempname ?"tmp" :"stat");
4073-
if (printed>len)
4073+
if (printed >=len)
40744074
elog(ERROR,"overlength pgstat path");
40754075
}
40764076

‎src/backend/utils/misc/guc.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9047,6 +9047,8 @@ do_serialize(char **destptr, Size *maxbytes, const char *fmt,...)
90479047
if (*maxbytes <=0)
90489048
elog(ERROR,"not enough space to serialize GUC state");
90499049

9050+
errno=0;
9051+
90509052
va_start(vargs,fmt);
90519053
n=vsnprintf(*destptr,*maxbytes,fmt,vargs);
90529054
va_end(vargs);

‎src/interfaces/ecpg/pgtypeslib/common.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ pgtypes_fmt_replace(union un_fmt_comb replace_val, int replace_type, char **outp
110110
break;
111111
}
112112

113-
if (i<0)
113+
if (i<0||i >=PGTYPES_FMT_NUM_MAX_DIGITS)
114114
{
115115
free(t);
116116
return-1;

‎src/port/getaddrinfo.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ getnameinfo(const struct sockaddr * sa, int salen,
404404
ret=snprintf(service,servicelen,"%d",
405405
ntohs(((structsockaddr_in*)sa)->sin_port));
406406
}
407-
if (ret==-1||ret >=servicelen)
407+
if (ret<0||ret >=servicelen)
408408
returnEAI_MEMORY;
409409
}
410410

‎src/test/isolation/isolation_main.c‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,27 @@ isolation_start_test(const char *testname,
7373
add_stringlist_item(expectfiles,expectfile);
7474

7575
if (launcher)
76+
{
7677
offset+=snprintf(psql_cmd+offset,sizeof(psql_cmd)-offset,
7778
"%s ",launcher);
79+
if (offset >=sizeof(psql_cmd))
80+
{
81+
fprintf(stderr,_("command too long\n"));
82+
exit(2);
83+
}
84+
}
7885

79-
snprintf(psql_cmd+offset,sizeof(psql_cmd)-offset,
80-
"\"%s\" \"dbname=%s\" < \"%s\" > \"%s\" 2>&1",
81-
isolation_exec,
82-
dblist->str,
83-
infile,
84-
outfile);
86+
offset+=snprintf(psql_cmd+offset,sizeof(psql_cmd)-offset,
87+
"\"%s\" \"dbname=%s\" < \"%s\" > \"%s\" 2>&1",
88+
isolation_exec,
89+
dblist->str,
90+
infile,
91+
outfile);
92+
if (offset >=sizeof(psql_cmd))
93+
{
94+
fprintf(stderr,_("command too long\n"));
95+
exit(2);
96+
}
8597

8698
pid=spawn_process(psql_cmd);
8799

‎src/test/regress/pg_regress.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -998,7 +998,7 @@ config_sspi_auth(const char *pgdata)
998998
} while (0)
999999

10001000
res=snprintf(fname,sizeof(fname),"%s/pg_hba.conf",pgdata);
1001-
if (res<0||res >=sizeof(fname)-1)
1001+
if (res<0||res >=sizeof(fname))
10021002
{
10031003
/*
10041004
* Truncating this name is a fatal error, because we must not fail to

‎src/test/regress/pg_regress_main.c‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,16 +60,28 @@ psql_start_test(const char *testname,
6060
add_stringlist_item(expectfiles,expectfile);
6161

6262
if (launcher)
63+
{
6364
offset+=snprintf(psql_cmd+offset,sizeof(psql_cmd)-offset,
6465
"%s ",launcher);
66+
if (offset >=sizeof(psql_cmd))
67+
{
68+
fprintf(stderr,_("command too long\n"));
69+
exit(2);
70+
}
71+
}
6572

66-
snprintf(psql_cmd+offset,sizeof(psql_cmd)-offset,
67-
"\"%s%spsql\" -X -a -q -d \"%s\" < \"%s\" > \"%s\" 2>&1",
68-
bindir ?bindir :"",
69-
bindir ?"/" :"",
70-
dblist->str,
71-
infile,
72-
outfile);
73+
offset+=snprintf(psql_cmd+offset,sizeof(psql_cmd)-offset,
74+
"\"%s%spsql\" -X -a -q -d \"%s\" < \"%s\" > \"%s\" 2>&1",
75+
bindir ?bindir :"",
76+
bindir ?"/" :"",
77+
dblist->str,
78+
infile,
79+
outfile);
80+
if (offset >=sizeof(psql_cmd))
81+
{
82+
fprintf(stderr,_("command too long\n"));
83+
exit(2);
84+
}
7385

7486
pid=spawn_process(psql_cmd);
7587

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp