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 707a3fc844c32638671aa61e8b564fbe6cb2e894..b9d7ab98c3e6f99d0d49ca1960f03027121315b2 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -2822,6 +2822,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 520e1338c28b6b188361e55bf31c0c65acf5735b..ca15b40939cb9a433d11098e58d6fcc550793245 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1281,6 +1281,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 396f79781c5e4e93399c9e82c6f2f7acd6c0f8df..64a60a260928dc50704eb54b276c08dc7cca2e01 100644 (file)
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -524,6 +524,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 6c75c8da6da63c9ba282a905eed1ecc3bb3c099a..26dfdde195a48813d58ccea69b5346d9a1ffd688 100644 (file)
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1450,6 +1450,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));
            }
@@ -4135,6 +4136,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, "SERVICE", PQservice(pset.db));
    SetVariable(pset.vars, "USER", PQuser(pset.db));

diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c
index 343e8618a56c9c04553d11d3565f5045600fdf06..7b8f7d4c52661d42b40e2e789d6c2f23cc7dc8f3 100644 (file)
--- a/src/bin/scripts/common.c
+++ b/src/bin/scripts/common.c
@@ -111,8 +111,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 b0e4747786274c02920548bae15769b44a12e1fc..7c0cf32d6a11b0ce7f2ec3705922aa07fc4ee897 100644 (file)
--- a/src/bin/scripts/createdb.c
+++ b/src/bin/scripts/createdb.c
@@ -198,6 +198,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 6aaf8332770445cead915cc3f4c4bf3885b27ff9..81e6abfc46e708dd9ce3b421f4bb96ca4d7462bc 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 ad762ffd43ab705bb189d0762108845b1d806be5..0b630818277130bf09fd0f48ad4f9402d4244754 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 8e5429a5cf1455839e72a665e483ad2e75b394a1..39bb16861736f7a2dc436e6378c0f784c7c1d9e0 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 076462a62f003b3f09c1c26c55de8eb3129a253b..665864fd22be0fa659445c6bfcc00435647510df 100644 (file)
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -511,7 +511,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;
    }
 
@@ -551,7 +552,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:
@@ -774,8 +776,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);
@@ -787,8 +790,9 @@ get_parallel_object_list(PGconn *conn, ReindexType type,
             * the order of tables list.
             */
            appendPQExpBufferStr(&buf,
-                                fmtQualifiedId(PQgetvalue(res, i, 1),
-                                               PQgetvalue(res, i, 2)));
+                                fmtQualifiedIdEnc(PQgetvalue(res, i, 1),
+                                                  PQgetvalue(res, i, 2),
+                                                  PQclientEncoding(conn)));
 
            simple_string_list_append(user_list, buf.data);
            resetPQExpBuffer(&buf);

diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 74fbc7ef0331699db52b1ef30f28179a069a05f3..982bf070be6f2cc543f654a66d8bacda4c258a82 100644 (file)
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -784,8 +784,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 8d5a3b4a6e4ab07112960eee59c1a28b2714c3c4..2945e28a2775c764936ec1392c0bca36e23905f7 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 690195fa9f9c1a6159286fbe2b23efc2dc2fcb44..dc991a932194ab951b7e91f2296bac2c0e10195f 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);