Skip to content

Commit 47458f8

Browse files
author
Neil Conway
committed
GCC 4.0 includes a new warning option, -Wformat-literal, that emits
a warning when a variable is used as a format string for printf() and similar functions (if the variable is derived from untrusted data, it could include unexpected formatting sequences). This emits too many warnings to be enabled by default, but it does flag a few dubious constructs in the Postgres tree. This patch fixes up the obvious variants: functions that are passed a variable format string but no additional arguments. Most of these are harmless (e.g. the ruleutils stuff), but there is at least one actual bug here: if you create a trigger named "%sfoo", pg_dump will read uninitialized memory and fail to dump the trigger correctly.
1 parent 16d4418 commit 47458f8

File tree

5 files changed

+22
-24
lines changed

5 files changed

+22
-24
lines changed

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* back to source text
44
*
55
* IDENTIFICATION
6-
* $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.193 2005/04/14 20:03:26 tgl Exp $
6+
* $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.194 2005/04/30 08:08:50 neilc Exp $
77
*
88
* This software is copyrighted by Jan Wieck - Hamburg.
99
*
@@ -733,7 +733,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno, int prettyFlags)
733733
AttrNumber attnum = idxrec->indkey.values[keyno];
734734

735735
if (!colno)
736-
appendStringInfo(&buf, sep);
736+
appendStringInfoString(&buf, sep);
737737
sep = ", ";
738738

739739
if (attnum != 0)
@@ -1885,7 +1885,7 @@ get_select_query_def(Query *query, deparse_context *context,
18851885
Oid sortcoltype;
18861886
TypeCacheEntry *typentry;
18871887

1888-
appendStringInfo(buf, sep);
1888+
appendStringInfoString(buf, sep);
18891889
sortexpr = get_rule_sortgroupclause(srt, query->targetList,
18901890
force_colno, context);
18911891
sortcoltype = exprType(sortexpr);
@@ -1954,7 +1954,7 @@ get_basic_select_query(Query *query, deparse_context *context,
19541954
{
19551955
SortClause *srt = (SortClause *) lfirst(l);
19561956

1957-
appendStringInfo(buf, sep);
1957+
appendStringInfoString(buf, sep);
19581958
get_rule_sortgroupclause(srt, query->targetList,
19591959
false, context);
19601960
sep = ", ";
@@ -1976,7 +1976,7 @@ get_basic_select_query(Query *query, deparse_context *context,
19761976
if (tle->resjunk)
19771977
continue; /* ignore junk entries */
19781978

1979-
appendStringInfo(buf, sep);
1979+
appendStringInfoString(buf, sep);
19801980
sep = ", ";
19811981
colno++;
19821982

@@ -2040,7 +2040,7 @@ get_basic_select_query(Query *query, deparse_context *context,
20402040
{
20412041
GroupClause *grp = (GroupClause *) lfirst(l);
20422042

2043-
appendStringInfo(buf, sep);
2043+
appendStringInfoString(buf, sep);
20442044
get_rule_sortgroupclause(grp, query->targetList,
20452045
false, context);
20462046
sep = ", ";
@@ -2229,7 +2229,7 @@ get_insert_query_def(Query *query, deparse_context *context)
22292229
if (tle->resjunk)
22302230
continue; /* ignore junk entries */
22312231

2232-
appendStringInfo(buf, sep);
2232+
appendStringInfoString(buf, sep);
22332233
sep = ", ";
22342234

22352235
/*
@@ -2301,7 +2301,7 @@ get_update_query_def(Query *query, deparse_context *context)
23012301
if (tle->resjunk)
23022302
continue; /* ignore junk entries */
23032303

2304-
appendStringInfo(buf, sep);
2304+
appendStringInfoString(buf, sep);
23052305
sep = ", ";
23062306

23072307
/*
@@ -3268,7 +3268,7 @@ get_rule_expr(Node *node, deparse_context *context,
32683268
if (tupdesc == NULL ||
32693269
!tupdesc->attrs[i]->attisdropped)
32703270
{
3271-
appendStringInfo(buf, sep);
3271+
appendStringInfoString(buf, sep);
32723272
get_rule_expr(e, context, true);
32733273
sep = ", ";
32743274
}
@@ -3280,7 +3280,7 @@ get_rule_expr(Node *node, deparse_context *context,
32803280
{
32813281
if (!tupdesc->attrs[i]->attisdropped)
32823282
{
3283-
appendStringInfo(buf, sep);
3283+
appendStringInfoString(buf, sep);
32843284
appendStringInfo(buf, "NULL");
32853285
sep = ", ";
32863286
}
@@ -3415,7 +3415,7 @@ get_rule_expr(Node *node, deparse_context *context,
34153415
sep = "";
34163416
foreach(l, (List *) node)
34173417
{
3418-
appendStringInfo(buf, sep);
3418+
appendStringInfoString(buf, sep);
34193419
get_rule_expr((Node *) lfirst(l), context, showimplicit);
34203420
sep = ", ";
34213421
}

‎src/bin/initdb/initdb.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
* Portions Copyright (c) 1994, Regents of the University of California
4040
* Portions taken from FreeBSD.
4141
*
42-
* $PostgreSQL: pgsql/src/bin/initdb/initdb.c,v 1.82 2005/04/28 21:47:16 tgl Exp $
42+
* $PostgreSQL: pgsql/src/bin/initdb/initdb.c,v 1.83 2005/04/30 08:08:51 neilc Exp $
4343
*
4444
*-------------------------------------------------------------------------
4545
*/
@@ -2609,7 +2609,7 @@ main(int argc, char *argv[])
26092609
make_template0();
26102610

26112611
if (authwarning != NULL)
2612-
fprintf(stderr, authwarning);
2612+
fprintf(stderr, "%s", authwarning);
26132613

26142614
/* Get directory specification used to start this executable */
26152615
strcpy(bin_dir, argv[0]);

‎src/bin/pg_dump/dumputils.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/bin/pg_dump/dumputils.c,v 1.16 2004/12/31 22:03:08 pgsql Exp $
10+
* $PostgreSQL: pgsql/src/bin/pg_dump/dumputils.c,v 1.17 2005/04/30 08:08:51 neilc Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -160,7 +160,7 @@ appendStringLiteralDQ(PQExpBuffer buf, const char *str, const char *dqprefix)
160160
/* start with $ + dqprefix if not NULL */
161161
appendPQExpBufferChar(delimBuf, '$');
162162
if (dqprefix)
163-
appendPQExpBuffer(delimBuf, dqprefix);
163+
appendPQExpBufferStr(delimBuf, dqprefix);
164164

165165
/*
166166
* Make sure we choose a delimiter which (without the trailing $) is

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*
1616
*
1717
* IDENTIFICATION
18-
* $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.107 2005/04/15 16:40:36 tgl Exp $
18+
* $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.108 2005/04/30 08:08:51 neilc Exp $
1919
*
2020
*-------------------------------------------------------------------------
2121
*/
@@ -345,7 +345,7 @@ RestoreArchive(Archive *AHX, RestoreOptions *ropt)
345345
* mode with libpq.
346346
*/
347347
if (te->copyStmt && strlen(te->copyStmt) > 0)
348-
ahprintf(AH, te->copyStmt);
348+
ahprintf(AH, "%s", te->copyStmt);
349349

350350
(*AH->PrintTocDataPtr) (AH, te, ropt);
351351

@@ -2197,9 +2197,7 @@ _reconnectToDB(ArchiveHandle *AH, const char *dbname)
21972197

21982198
appendPQExpBuffer(qry, "\\connect %s\n\n",
21992199
dbname ? fmtId(dbname) : "-");
2200-
2201-
ahprintf(AH, qry->data);
2202-
2200+
ahprintf(AH, "%s", qry->data);
22032201
destroyPQExpBuffer(qry);
22042202
}
22052203

‎src/bin/pg_dump/pg_dump.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
* by PostgreSQL
1313
*
1414
* IDENTIFICATION
15-
* $PostgreSQL: pgsql/src/bin/pg_dump/pg_dump.c,v 1.407 2005/04/15 16:40:36 tgl Exp $
15+
* $PostgreSQL: pgsql/src/bin/pg_dump/pg_dump.c,v 1.408 2005/04/30 08:08:51 neilc Exp $
1616
*
1717
*-------------------------------------------------------------------------
1818
*/
@@ -976,7 +976,7 @@ dumpTableData_insert(Archive *fout, void *dcontext)
976976
{
977977
if (field > 0)
978978
appendPQExpBuffer(q, ", ");
979-
appendPQExpBuffer(q, fmtId(PQfname(res, field)));
979+
appendPQExpBufferStr(q, fmtId(PQfname(res, field)));
980980
}
981981
appendPQExpBuffer(q, ") ");
982982
archputs(q->data, fout);
@@ -7599,12 +7599,12 @@ dumpTrigger(Archive *fout, TriggerInfo *tginfo)
75997599
if (tginfo->tgisconstraint)
76007600
{
76017601
appendPQExpBuffer(query, "CREATE CONSTRAINT TRIGGER ");
7602-
appendPQExpBuffer(query, fmtId(tginfo->tgconstrname));
7602+
appendPQExpBufferStr(query, fmtId(tginfo->tgconstrname));
76037603
}
76047604
else
76057605
{
76067606
appendPQExpBuffer(query, "CREATE TRIGGER ");
7607-
appendPQExpBuffer(query, fmtId(tginfo->dobj.name));
7607+
appendPQExpBufferStr(query, fmtId(tginfo->dobj.name));
76087608
}
76097609
appendPQExpBuffer(query, "\n ");
76107610

0 commit comments

Comments
 (0)