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 ea979b50b0078102cac91ed75bc2e7ae9cb8daeb..55b24f1837f598b4737b5f4d75f8c4b168ab0b01 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -2733,6 +2733,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 2f3bd13039012396a96f92f49f77a2827348ca55..52c9ff847fd9489b36a9d4b69469296aac72e106 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1095,6 +1095,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 950bc41380d558ce07d6fc2138d14ed21506305b..623cc6c1a903535d93e0e4ff0a526402c41bc01b 100644 (file)
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -494,6 +494,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 b9fd475eb91f8b88b1162e68b8cf21600ec348ca..57cef4fa670c3f0a41465614fe4334fe41d1fe47 100644 (file)
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1285,6 +1285,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));
                        }
@@ -3740,6 +3741,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 9e38b60a6552e5d48366a2e3fe7d08345db76f0c..e92b586c96bec65c5f47ae99f295eb281e519ddb 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 cbeef4f54f0d217c682b4120daa98faedf02ded4..e7859b80e138df346189358d35e9c3383a48a3a8 100644 (file)
--- a/src/bin/scripts/createdb.c
+++ b/src/bin/scripts/createdb.c
@@ -196,6 +196,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 bfba0d09d1174bf47dec2748a51f9d3a9f9bbb66..28436a2a2731783fd88b493031ac13b6d05c9597 100644 (file)
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -261,6 +261,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 afc00dac7841b7bf0d1b3affd543fe033ea6452d..5f183579f1092b67050ec44ebacec865a85556db 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 82c1f35ab23fef2c8c440751087aee32540a587f..aaee4437d16bc4c4b359db4d951a2a036ea9f381 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 eb1e6fe825ac621bb68bbdf56f5097eaa8494001..8db37a72306ca25150c9805d679c3f215547bd4e 100644 (file)
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -501,7 +501,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;
        }
 
@@ -541,7 +542,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:
@@ -711,8 +713,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 cc43141f0a0433f6e6ff771a869fe09aaa34bb63..425b505afe22e2a3fa1949ab85938ef3b34e53e1 100644 (file)
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -646,8 +646,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 (tables_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 f311bdd3ad5b7b5bc9484727ea294ba7c2680662..d3640f8ec1d26d27ce310c64b0b83190ae9af68c 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 fa4deb24978d102f5bfa66808eb5196fe5bb15bf..92b2b2d3083fb9189f9505d00383b0c82c22ecbd 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 *identifier);
+extern const char *fmtIdEnc(const char *identifier, 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);