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

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 958f88d4201212deae313023a5c7712212b44c28..3448aba12f47b04f806e351d9076ea720730ab7f 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -2713,6 +2713,7 @@ processEncodingEntry(ArchiveHandle *AH, TocEntry *te)
                        pg_fatal("unrecognized encoding \"%s\"",
                                         ptr1);
                AH->public.encoding = encoding;
+               setFmtEncoding(encoding);
        }
        else
                pg_fatal("invalid ENCODING item: %s",

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index c0d22b87af464a5095f83bff6f8e21c3db1d3399..ba71c0b0f3246b67be45acb3f1b6be54d8432031 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1172,6 +1172,7 @@ setup_connection(Archive *AH, const char *dumpencoding,
         * we know how to escape strings.
         */
        AH->encoding = PQclientEncoding(conn);
+       setFmtEncoding(AH->encoding);
 
        std_strings = PQparameterStatus(conn, "standard_conforming_strings");
        AH->std_strings = (std_strings && strcmp(std_strings, "on") == 0);

diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 591fcb07c5b16c68d4face76eb540a1bd8ce5f7f..9d8732ac736673536c67b5c7f949c5eec33b6b9b 100644 (file)
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -519,6 +519,7 @@ main(int argc, char *argv[])
         * we know how to escape strings.
         */
        encoding = PQclientEncoding(conn);
+       setFmtEncoding(encoding);
        std_strings = PQparameterStatus(conn, "standard_conforming_strings");
        if (!std_strings)
                std_strings = "off";

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index e2e639ce76d8ef918bbca2cc0d5e0139fc0b1dfa..0cf32d156a8d86a6a904dbe6fafb0c4072819358 100644 (file)
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1318,6 +1318,7 @@ exec_command_encoding(PsqlScanState scan_state, bool active_branch)
                                /* save encoding info into psql internal data */
                                pset.encoding = PQclientEncoding(pset.db);
                                pset.popt.topt.encoding = pset.encoding;
+                               setFmtEncoding(pset.encoding);
                                SetVariable(pset.vars, "ENCODING",
                                                        pg_encoding_to_char(pset.encoding));
                        }
@@ -3867,6 +3868,8 @@ SyncVariables(void)
        pset.popt.topt.encoding = pset.encoding;
        pset.sversion = PQserverVersion(pset.db);
 
+       setFmtEncoding(pset.encoding);
+
        SetVariable(pset.vars, "DBNAME", PQdb(pset.db));
        SetVariable(pset.vars, "USER", PQuser(pset.db));
        SetVariable(pset.vars, "HOST", PQhost(pset.db));

diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c
index 4c52fb6a3e436b9229484573b8b89dd92713dfe2..c7a607f7afdde30cc86c1081d2e505ac8e5c3099 100644 (file)
--- a/src/bin/scripts/common.c
+++ b/src/bin/scripts/common.c
@@ -112,8 +112,9 @@ appendQualifiedRelation(PQExpBuffer buf, const char *spec,
                exit(1);
        }
        appendPQExpBufferStr(buf,
-                                                fmtQualifiedId(PQgetvalue(res, 0, 1),
-                                                                               PQgetvalue(res, 0, 0)));
+                                                fmtQualifiedIdEnc(PQgetvalue(res, 0, 1),
+                                                                                  PQgetvalue(res, 0, 0),
+                                                                                  PQclientEncoding(conn)));
        appendPQExpBufferStr(buf, columns);
        PQclear(res);
        termPQExpBuffer(&sql);

diff --git a/src/bin/scripts/createdb.c b/src/bin/scripts/createdb.c
index ef34b24890cd1309ce989e16947f51c89cbc0384..05636883256870520497aef98768832756dff384 100644 (file)
--- a/src/bin/scripts/createdb.c
+++ b/src/bin/scripts/createdb.c
@@ -193,6 +193,8 @@ main(int argc, char *argv[])
 
        conn = connectMaintenanceDatabase(&cparams, progname, echo);
 
+       setFmtEncoding(PQclientEncoding(conn));
+
        initPQExpBuffer(&sql);
 
        appendPQExpBuffer(&sql, "CREATE DATABASE %s",

diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index 0709491185c39eb6d7e474ee7e7c39134ace62fe..e69301501fe6e727e1aba6a0631568b7fa4abad0 100644 (file)
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -292,6 +292,8 @@ main(int argc, char *argv[])
 
        conn = connectMaintenanceDatabase(&cparams, progname, echo);
 
+       setFmtEncoding(PQclientEncoding(conn));
+
        initPQExpBuffer(&sql);
 
        printfPQExpBuffer(&sql, "CREATE ROLE %s", fmtId(newuser));

diff --git a/src/bin/scripts/dropdb.c b/src/bin/scripts/dropdb.c
index 8d0f432f3d7c73c3bfdde52541941077bfbd6e41..483f11cefffe5ca87691d6d24e5eabe67e1a8e58 100644 (file)
--- a/src/bin/scripts/dropdb.c
+++ b/src/bin/scripts/dropdb.c
@@ -129,13 +129,6 @@ main(int argc, char *argv[])
                        exit(0);
        }
 
-       initPQExpBuffer(&sql);
-
-       appendPQExpBuffer(&sql, "DROP DATABASE %s%s%s;",
-                                         (if_exists ? "IF EXISTS " : ""),
-                                         fmtId(dbname),
-                                         force ? " WITH (FORCE)" : "");
-
        /* Avoid trying to drop postgres db while we are connected to it. */
        if (maintenance_db == NULL && strcmp(dbname, "postgres") == 0)
                maintenance_db = "template1";
@@ -149,6 +142,12 @@ main(int argc, char *argv[])
 
        conn = connectMaintenanceDatabase(&cparams, progname, echo);
 
+       initPQExpBuffer(&sql);
+       appendPQExpBuffer(&sql, "DROP DATABASE %s%s%s;",
+                                         (if_exists ? "IF EXISTS " : ""),
+                                         fmtIdEnc(dbname, PQclientEncoding(conn)),
+                                         force ? " WITH (FORCE)" : "");
+
        if (echo)
                printf("%s\n", sql.data);
        result = PQexec(conn, sql.data);

diff --git a/src/bin/scripts/dropuser.c b/src/bin/scripts/dropuser.c
index 8aad932e6a5bbd0565cc12fb4f8e03865580683f..30fcdec912ad82cdb6dcc53554252d582318e7eb 100644 (file)
--- a/src/bin/scripts/dropuser.c
+++ b/src/bin/scripts/dropuser.c
@@ -143,7 +143,8 @@ main(int argc, char *argv[])
 
        initPQExpBuffer(&sql);
        appendPQExpBuffer(&sql, "DROP ROLE %s%s;",
-                                         (if_exists ? "IF EXISTS " : ""), fmtId(dropuser));
+                                         (if_exists ? "IF EXISTS " : ""),
+                                         fmtIdEnc(dropuser, PQclientEncoding(conn)));
 
        if (echo)
                printf("%s\n", sql.data);

diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index b3becff8eca62234e80784613ccbe9114cc5bdca..93fb479d9774950e3eaf96355be8bda8f3b58c1b 100644 (file)
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -489,7 +489,8 @@ run_reindex_command(PGconn *conn, ReindexType type, const char *name,
 
        if (tablespace)
        {
-               appendPQExpBuffer(&sql, "%sTABLESPACE %s", sep, fmtId(tablespace));
+               appendPQExpBuffer(&sql, "%sTABLESPACE %s", sep,
+                                                 fmtIdEnc(tablespace, PQclientEncoding(conn)));
                sep = comma;
        }
 
@@ -529,7 +530,8 @@ run_reindex_command(PGconn *conn, ReindexType type, const char *name,
        {
                case REINDEX_DATABASE:
                case REINDEX_SYSTEM:
-                       appendPQExpBufferStr(&sql, fmtId(name));
+                       appendPQExpBufferStr(&sql,
+                                                                fmtIdEnc(name, PQclientEncoding(conn)));
                        break;
                case REINDEX_INDEX:
                case REINDEX_TABLE:
@@ -699,8 +701,9 @@ get_parallel_object_list(PGconn *conn, ReindexType type,
        for (i = 0; i < ntups; i++)
        {
                appendPQExpBufferStr(&buf,
-                                                        fmtQualifiedId(PQgetvalue(res, i, 1),
-                                                                                       PQgetvalue(res, i, 0)));
+                                                        fmtQualifiedIdEnc(PQgetvalue(res, i, 1),
+                                                                                          PQgetvalue(res, i, 0),
+                                                                                          PQclientEncoding(conn)));
 
                simple_string_list_append(tables, buf.data);
                resetPQExpBuffer(&buf);

diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index c475c0f28612012d53c0cccd79d2076613ed9e2e..a1ebabc0735117521da680ae27882f8dbe9f7815 100644 (file)
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -795,8 +795,9 @@ vacuum_one_database(ConnParams *cparams,
        for (i = 0; i < ntups; i++)
        {
                appendPQExpBufferStr(&buf,
-                                                        fmtQualifiedId(PQgetvalue(res, i, 1),
-                                                                                       PQgetvalue(res, i, 0)));
+                                                        fmtQualifiedIdEnc(PQgetvalue(res, i, 1),
+                                                                                          PQgetvalue(res, i, 0),
+                                                                                          PQclientEncoding(conn)));
 
                if (objects_listed && !PQgetisnull(res, i, 2))
                        appendPQExpBufferStr(&buf, PQgetvalue(res, i, 2));

diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c
index 0429a72bfe245054cca6f7e40bc42bce93c30aa9..2e2b0d18af4319a5b1f0e69245eed78bcdf6a9cc 100644 (file)
--- a/src/fe_utils/string_utils.c
+++ b/src/fe_utils/string_utils.c
@@ -19,6 +19,7 @@
 
 #include "common/keywords.h"
 #include "fe_utils/string_utils.h"
+#include "mb/pg_wchar.h"
 
 static PQExpBuffer defaultGetLocalPQExpBuffer(void);
 
@@ -26,6 +27,8 @@ static PQExpBuffer defaultGetLocalPQExpBuffer(void);
 int                    quote_all_identifiers = 0;
 PQExpBuffer (*getLocalPQExpBuffer) (void) = defaultGetLocalPQExpBuffer;
 
+static int     fmtIdEncoding = -1;
+
 
 /*
  * Returns a temporary PQExpBuffer, valid until the next call to the function.
@@ -54,14 +57,48 @@ defaultGetLocalPQExpBuffer(void)
        return id_return;
 }
 
+/*
+ * Set the encoding that fmtId() and fmtQualifiedId() use.
+ *
+ * This is not safe against multiple connections having different encodings,
+ * but there is no real other way to address the need to know the encoding for
+ * fmtId()/fmtQualifiedId() input for safe escaping. Eventually we should get
+ * rid of fmtId().
+ */
+void
+setFmtEncoding(int encoding)
+{
+       fmtIdEncoding = encoding;
+}
+
+/*
+ * Return the currently configured encoding for fmtId() and fmtQualifiedId().
+ */
+static int
+getFmtEncoding(void)
+{
+       if (fmtIdEncoding != -1)
+               return fmtIdEncoding;
+
+       /*
+        * In assertion builds it seems best to fail hard if the encoding was not
+        * set, to make it easier to find places with missing calls. But in
+        * production builds that seems like a bad idea, thus we instead just
+        * default to UTF-8.
+        */
+       Assert(fmtIdEncoding != -1);
+
+       return PG_UTF8;
+}
+
 /*
  *     Quotes input string if it's not a legitimate SQL identifier as-is.
  *
- *     Note that the returned string must be used before calling fmtId again,
+ *     Note that the returned string must be used before calling fmtIdEnc again,
  *     since we re-use the same return buffer each time.
  */
 const char *
-fmtId(const char *rawid)
+fmtIdEnc(const char *rawid, int encoding)
 {
        PQExpBuffer id_return = getLocalPQExpBuffer();
 
@@ -134,7 +171,24 @@ fmtId(const char *rawid)
 }
 
 /*
- * fmtQualifiedId - construct a schema-qualified name, with quoting as needed.
+ *     Quotes input string if it's not a legitimate SQL identifier as-is.
+ *
+ *     Note that the returned string must be used before calling fmtId again,
+ *     since we re-use the same return buffer each time.
+ *
+ *  NB: This assumes setFmtEncoding() previously has been called to configure
+ *  the encoding of rawid. It is preferable to use fmtIdEnc() with an
+ *  explicit encoding.
+ */
+const char *
+fmtId(const char *rawid)
+{
+       return fmtIdEnc(rawid, getFmtEncoding());
+}
+
+/*
+ * fmtQualifiedIdEnc - construct a schema-qualified name, with quoting as
+ * needed.
  *
  * Like fmtId, use the result before calling again.
  *
@@ -142,7 +196,7 @@ fmtId(const char *rawid)
  * use that buffer until we're finished with calling fmtId().
  */
 const char *
-fmtQualifiedId(const char *schema, const char *id)
+fmtQualifiedIdEnc(const char *schema, const char *id, int encoding)
 {
        PQExpBuffer id_return;
        PQExpBuffer lcl_pqexp = createPQExpBuffer();
@@ -150,9 +204,9 @@ fmtQualifiedId(const char *schema, const char *id)
        /* Some callers might fail to provide a schema name */
        if (schema && *schema)
        {
-               appendPQExpBuffer(lcl_pqexp, "%s.", fmtId(schema));
+               appendPQExpBuffer(lcl_pqexp, "%s.", fmtIdEnc(schema, encoding));
        }
-       appendPQExpBufferStr(lcl_pqexp, fmtId(id));
+       appendPQExpBufferStr(lcl_pqexp, fmtIdEnc(id, encoding));
 
        id_return = getLocalPQExpBuffer();
 
@@ -162,6 +216,24 @@ fmtQualifiedId(const char *schema, const char *id)
        return id_return->data;
 }
 
+/*
+ * fmtQualifiedId - construct a schema-qualified name, with quoting as needed.
+ *
+ * Like fmtId, use the result before calling again.
+ *
+ * Since we call fmtId and it also uses getLocalPQExpBuffer() we cannot
+ * use that buffer until we're finished with calling fmtId().
+ *
+ * NB: This assumes setFmtEncoding() previously has been called to configure
+ * the encoding of schema/id. It is preferable to use fmtQualifiedIdEnc()
+ * with an explicit encoding.
+ */
+const char *
+fmtQualifiedId(const char *schema, const char *id)
+{
+       return fmtQualifiedIdEnc(schema, id, getFmtEncoding());
+}
+
 
 /*
  * Format a Postgres version number (in the PG_VERSION_NUM integer format

diff --git a/src/include/fe_utils/string_utils.h b/src/include/fe_utils/string_utils.h
index e10c9090754cc2236279c79f4a55e1723af1bcb6..9980f19316150b28fd53d8a16aa53f2c1b3c7263 100644 (file)
--- a/src/include/fe_utils/string_utils.h
+++ b/src/include/fe_utils/string_utils.h
@@ -25,7 +25,10 @@ extern PQExpBuffer (*getLocalPQExpBuffer) (void);
 
 /* Functions */
 extern const char *fmtId(const char *rawid);
+extern const char *fmtIdEnc(const char *rawid, int encoding);
 extern const char *fmtQualifiedId(const char *schema, const char *id);
+extern const char *fmtQualifiedIdEnc(const char *schema, const char *id, int encoding);
+extern void setFmtEncoding(int encoding);
 
 extern char *formatPGVersionNumber(int version_number, bool include_minor,
                                                                   char *buf, size_t buflen);