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 8c20c263c4b45f819c7af4f89135bac6d9ef8adf..f1ffed038b046b5262756b87143ac6ef2fcd4744 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -2818,6 +2818,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 d2b0667a2a9c5dbcda6345ed92f7ca43c65c2528..85adb5dee7654a0676355324c81585df1ab85e9d 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1211,6 +1211,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 e3ad8fb295683bce66a9c1a5715de5f8a27d0544..c0b8467a2fc72e7666a5e09d206b4ea9166969b2 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 be048669a3f16373bdd82b8c3ba1e71da429a6d9..f3f8fd0765ac713cb6d296dc041bd380775d8f38 100644 (file)
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1336,6 +1336,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));
            }
@@ -3956,6 +3957,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 d16381eda903cfe252ac520a5d2c134c47328f74..4b0e26c0bb13a5792b0a1f9fe823cf06557bbad0 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 30426860efae464e61d6df3f65b7b708c0475b43..2fa8477e0ed8bdea0794b1dbef3ef4d9046215cf 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 f5f03c1145fb84d305159fa2ea5135f8bea2e747..9c1f5d23392c9f72ebc5ea81c2fb8594ab769464 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 57656ba8988aba7d4f25f80c66bf6160d22ec162..435837b65ffbc638416774b47a9230901e3dc5f7 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 233ccd288b7fd70aa9aab09020efc65627058bb8..c6357ce10bba26d5fbcdd64275fa0fe29f379a54 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 af0738d933496287e3a18de08321e6f24d11e9da..e736b2baccc06f01149837d13c032e8e6b331d43 100644 (file)
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -512,7 +512,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;
    }
 
@@ -552,7 +553,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:
@@ -775,8 +777,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);
@@ -788,8 +791,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 430582cef5343a91a9d4541e509130805a49ac75..5aff0dd25d5a2a347d07b3eced0f8f779a4fcf34 100644 (file)
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -785,8 +785,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 09fd33907ddc8206a08a70a28e5f8515da6fed23..3000da913da4e2b58b73ec473ea6481e6b878a46 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 f150a7f7ef7d8d88ade4ccf45a99c64ccaf2b604..500f585a4a5858e426bceb9cbf795299e2259279 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);