Skip to content

Commit 5df4e16

Browse files
anarazelanarazel
committed
Specify the encoding of input to fmtId()
This commit adds fmtIdEnc() and fmtQualifiedIdEnc(), which allow to specify the encoding as an explicit argument. Additionally setFmtEncoding() is provided, which defines the encoding when no explicit encoding is provided, to avoid breaking all code using fmtId(). All users of fmtId()/fmtQualifiedId() are either converted to the explicit version or a call to setFmtEncoding() has been added. This commit does not yet utilize the now well-defined encoding, that will happen in a subsequent commit. Reviewed-by: Noah Misch <noah@leadboat.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Backpatch-through: 13 Security: CVE-2025-1094
1 parent db3eb0e commit 5df4e16

File tree

13 files changed

+110
-21
lines changed

13 files changed

+110
-21
lines changed

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2725,6 +2725,7 @@ processEncodingEntry(ArchiveHandle *AH, TocEntry *te)
27252725
fatal("unrecognized encoding \"%s\"",
27262726
ptr1);
27272727
AH->public.encoding = encoding;
2728+
setFmtEncoding(encoding);
27282729
}
27292730
else
27302731
fatal("invalid ENCODING item: %s",

‎src/bin/pg_dump/pg_dump.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,6 +1094,7 @@ setup_connection(Archive *AH, const char *dumpencoding,
10941094
* we know how to escape strings.
10951095
*/
10961096
AH->encoding = PQclientEncoding(conn);
1097+
setFmtEncoding(AH->encoding);
10971098

10981099
std_strings = PQparameterStatus(conn, "standard_conforming_strings");
10991100
AH->std_strings = (std_strings && strcmp(std_strings, "on") == 0);

‎src/bin/pg_dump/pg_dumpall.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,7 @@ main(int argc, char *argv[])
507507
* we know how to escape strings.
508508
*/
509509
encoding = PQclientEncoding(conn);
510+
setFmtEncoding(encoding);
510511
std_strings = PQparameterStatus(conn, "standard_conforming_strings");
511512
if (!std_strings)
512513
std_strings = "off";

‎src/bin/psql/command.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,6 +1220,7 @@ exec_command_encoding(PsqlScanState scan_state, bool active_branch)
12201220
/* save encoding info into psql internal data */
12211221
pset.encoding = PQclientEncoding(pset.db);
12221222
pset.popt.topt.encoding = pset.encoding;
1223+
setFmtEncoding(pset.encoding);
12231224
SetVariable(pset.vars, "ENCODING",
12241225
pg_encoding_to_char(pset.encoding));
12251226
}
@@ -3606,6 +3607,8 @@ SyncVariables(void)
36063607
pset.popt.topt.encoding = pset.encoding;
36073608
pset.sversion = PQserverVersion(pset.db);
36083609

3610+
setFmtEncoding(pset.encoding);
3611+
36093612
SetVariable(pset.vars, "DBNAME", PQdb(pset.db));
36103613
SetVariable(pset.vars, "USER", PQuser(pset.db));
36113614
SetVariable(pset.vars, "HOST", PQhost(pset.db));

‎src/bin/scripts/common.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,8 +430,9 @@ appendQualifiedRelation(PQExpBuffer buf, const char *spec,
430430
exit(1);
431431
}
432432
appendPQExpBufferStr(buf,
433-
fmtQualifiedId(PQgetvalue(res, 0, 1),
434-
PQgetvalue(res, 0, 0)));
433+
fmtQualifiedIdEnc(PQgetvalue(res, 0, 1),
434+
PQgetvalue(res, 0, 0),
435+
PQclientEncoding(conn)));
435436
appendPQExpBufferStr(buf, columns);
436437
PQclear(res);
437438
termPQExpBuffer(&sql);

‎src/bin/scripts/createdb.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,8 @@ main(int argc, char *argv[])
190190

191191
conn = connectMaintenanceDatabase(&cparams, progname, echo);
192192

193+
setFmtEncoding(PQclientEncoding(conn));
194+
193195
initPQExpBuffer(&sql);
194196

195197
appendPQExpBuffer(&sql, "CREATE DATABASE %s",

‎src/bin/scripts/createuser.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,8 @@ main(int argc, char *argv[])
266266

267267
conn = connectMaintenanceDatabase(&cparams, progname, echo);
268268

269+
setFmtEncoding(PQclientEncoding(conn));
270+
269271
initPQExpBuffer(&sql);
270272

271273
printfPQExpBuffer(&sql, "CREATE ROLE %s", fmtId(newuser));

‎src/bin/scripts/dropdb.c

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,13 +127,6 @@ main(int argc, char *argv[])
127127
exit(0);
128128
}
129129

130-
initPQExpBuffer(&sql);
131-
132-
appendPQExpBuffer(&sql, "DROP DATABASE %s%s%s;",
133-
(if_exists ? "IF EXISTS " : ""),
134-
fmtId(dbname),
135-
force ? " WITH (FORCE)" : "");
136-
137130
/* Avoid trying to drop postgres db while we are connected to it. */
138131
if (maintenance_db == NULL && strcmp(dbname, "postgres") == 0)
139132
maintenance_db = "template1";
@@ -147,6 +140,12 @@ main(int argc, char *argv[])
147140

148141
conn = connectMaintenanceDatabase(&cparams, progname, echo);
149142

143+
initPQExpBuffer(&sql);
144+
appendPQExpBuffer(&sql, "DROP DATABASE %s%s%s;",
145+
(if_exists ? "IF EXISTS " : ""),
146+
fmtIdEnc(dbname, PQclientEncoding(conn)),
147+
force ? " WITH (FORCE)" : "");
148+
150149
if (echo)
151150
printf("%s\n", sql.data);
152151
result = PQexec(conn, sql.data);

‎src/bin/scripts/dropuser.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ main(int argc, char *argv[])
143143

144144
initPQExpBuffer(&sql);
145145
appendPQExpBuffer(&sql, "DROP ROLE %s%s;",
146-
(if_exists ? "IF EXISTS " : ""), fmtId(dropuser));
146+
(if_exists ? "IF EXISTS " : ""),
147+
fmtIdEnc(dropuser, PQclientEncoding(conn)));
147148

148149
if (echo)
149150
printf("%s\n", sql.data);

‎src/bin/scripts/reindexdb.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,8 @@ run_reindex_command(PGconn *conn, ReindexType type, const char *name,
532532
{
533533
case REINDEX_DATABASE:
534534
case REINDEX_SYSTEM:
535-
appendPQExpBufferStr(&sql, fmtId(name));
535+
appendPQExpBufferStr(&sql,
536+
fmtIdEnc(name, PQclientEncoding(conn)));
536537
break;
537538
case REINDEX_INDEX:
538539
case REINDEX_TABLE:
@@ -702,8 +703,9 @@ get_parallel_object_list(PGconn *conn, ReindexType type,
702703
for (i = 0; i < ntups; i++)
703704
{
704705
appendPQExpBufferStr(&buf,
705-
fmtQualifiedId(PQgetvalue(res, i, 1),
706-
PQgetvalue(res, i, 0)));
706+
fmtQualifiedIdEnc(PQgetvalue(res, i, 1),
707+
PQgetvalue(res, i, 0),
708+
PQclientEncoding(conn)));
707709

708710
simple_string_list_append(tables, buf.data);
709711
resetPQExpBuffer(&buf);

‎src/bin/scripts/vacuumdb.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -611,8 +611,9 @@ vacuum_one_database(const ConnParams *cparams,
611611
for (i = 0; i < ntups; i++)
612612
{
613613
appendPQExpBufferStr(&buf,
614-
fmtQualifiedId(PQgetvalue(res, i, 1),
615-
PQgetvalue(res, i, 0)));
614+
fmtQualifiedIdEnc(PQgetvalue(res, i, 1),
615+
PQgetvalue(res, i, 0),
616+
PQclientEncoding(conn)));
616617

617618
if (tables_listed && !PQgetisnull(res, i, 2))
618619
appendPQExpBufferStr(&buf, PQgetvalue(res, i, 2));

‎src/fe_utils/string_utils.c

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,16 @@
1919

2020
#include "common/keywords.h"
2121
#include "fe_utils/string_utils.h"
22+
#include "mb/pg_wchar.h"
2223

2324
static PQExpBuffer defaultGetLocalPQExpBuffer(void);
2425

2526
/* Globals exported by this file */
2627
int quote_all_identifiers = 0;
2728
PQExpBuffer (*getLocalPQExpBuffer) (void) = defaultGetLocalPQExpBuffer;
2829

30+
static int fmtIdEncoding = -1;
31+
2932

3033
/*
3134
* Returns a temporary PQExpBuffer, valid until the next call to the function.
@@ -54,14 +57,48 @@ defaultGetLocalPQExpBuffer(void)
5457
return id_return;
5558
}
5659

60+
/*
61+
* Set the encoding that fmtId() and fmtQualifiedId() use.
62+
*
63+
* This is not safe against multiple connections having different encodings,
64+
* but there is no real other way to address the need to know the encoding for
65+
* fmtId()/fmtQualifiedId() input for safe escaping. Eventually we should get
66+
* rid of fmtId().
67+
*/
68+
void
69+
setFmtEncoding(int encoding)
70+
{
71+
fmtIdEncoding = encoding;
72+
}
73+
74+
/*
75+
* Return the currently configured encoding for fmtId() and fmtQualifiedId().
76+
*/
77+
static int
78+
getFmtEncoding(void)
79+
{
80+
if (fmtIdEncoding != -1)
81+
return fmtIdEncoding;
82+
83+
/*
84+
* In assertion builds it seems best to fail hard if the encoding was not
85+
* set, to make it easier to find places with missing calls. But in
86+
* production builds that seems like a bad idea, thus we instead just
87+
* default to UTF-8.
88+
*/
89+
Assert(fmtIdEncoding != -1);
90+
91+
return PG_UTF8;
92+
}
93+
5794
/*
5895
* Quotes input string if it's not a legitimate SQL identifier as-is.
5996
*
60-
* Note that the returned string must be used before calling fmtId again,
97+
* Note that the returned string must be used before calling fmtIdEnc again,
6198
* since we re-use the same return buffer each time.
6299
*/
63100
const char *
64-
fmtId(const char *rawid)
101+
fmtIdEnc(const char *rawid, int encoding)
65102
{
66103
PQExpBuffer id_return = getLocalPQExpBuffer();
67104

@@ -134,25 +171,42 @@ fmtId(const char *rawid)
134171
}
135172

136173
/*
137-
* fmtQualifiedId - construct a schema-qualified name, with quoting as needed.
174+
* Quotes input string if it's not a legitimate SQL identifier as-is.
175+
*
176+
* Note that the returned string must be used before calling fmtId again,
177+
* since we re-use the same return buffer each time.
178+
*
179+
* NB: This assumes setFmtEncoding() previously has been called to configure
180+
* the encoding of rawid. It is preferable to use fmtIdEnc() with an
181+
* explicit encoding.
182+
*/
183+
const char *
184+
fmtId(const char *rawid)
185+
{
186+
return fmtIdEnc(rawid, getFmtEncoding());
187+
}
188+
189+
/*
190+
* fmtQualifiedIdEnc - construct a schema-qualified name, with quoting as
191+
* needed.
138192
*
139193
* Like fmtId, use the result before calling again.
140194
*
141195
* Since we call fmtId and it also uses getLocalPQExpBuffer() we cannot
142196
* use that buffer until we're finished with calling fmtId().
143197
*/
144198
const char *
145-
fmtQualifiedId(const char *schema, const char *id)
199+
fmtQualifiedIdEnc(const char *schema, const char *id, int encoding)
146200
{
147201
PQExpBuffer id_return;
148202
PQExpBuffer lcl_pqexp = createPQExpBuffer();
149203

150204
/* Some callers might fail to provide a schema name */
151205
if (schema && *schema)
152206
{
153-
appendPQExpBuffer(lcl_pqexp, "%s.", fmtId(schema));
207+
appendPQExpBuffer(lcl_pqexp, "%s.", fmtIdEnc(schema, encoding));
154208
}
155-
appendPQExpBufferStr(lcl_pqexp, fmtId(id));
209+
appendPQExpBufferStr(lcl_pqexp, fmtIdEnc(id, encoding));
156210

157211
id_return = getLocalPQExpBuffer();
158212

@@ -162,6 +216,24 @@ fmtQualifiedId(const char *schema, const char *id)
162216
return id_return->data;
163217
}
164218

219+
/*
220+
* fmtQualifiedId - construct a schema-qualified name, with quoting as needed.
221+
*
222+
* Like fmtId, use the result before calling again.
223+
*
224+
* Since we call fmtId and it also uses getLocalPQExpBuffer() we cannot
225+
* use that buffer until we're finished with calling fmtId().
226+
*
227+
* NB: This assumes setFmtEncoding() previously has been called to configure
228+
* the encoding of schema/id. It is preferable to use fmtQualifiedIdEnc()
229+
* with an explicit encoding.
230+
*/
231+
const char *
232+
fmtQualifiedId(const char *schema, const char *id)
233+
{
234+
return fmtQualifiedIdEnc(schema, id, getFmtEncoding());
235+
}
236+
165237

166238
/*
167239
* Format a Postgres version number (in the PG_VERSION_NUM integer format

‎src/include/fe_utils/string_utils.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ extern PQExpBuffer (*getLocalPQExpBuffer) (void);
2525

2626
/* Functions */
2727
extern const char *fmtId(const char *identifier);
28+
extern const char *fmtIdEnc(const char *identifier, int encoding);
2829
extern const char *fmtQualifiedId(const char *schema, const char *id);
30+
extern const char *fmtQualifiedIdEnc(const char *schema, const char *id, int encoding);
31+
extern void setFmtEncoding(int encoding);
2932

3033
extern char *formatPGVersionNumber(int version_number, bool include_minor,
3134
char *buf, size_t buflen);

0 commit comments

Comments
 (0)