Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Specify the encoding of input to fmtId()
authorAndres Freund <andres@anarazel.de>
Mon, 10 Feb 2025 15:03:40 +0000 (10:03 -0500)
committerAndres Freund <andres@anarazel.de>
Mon, 10 Feb 2025 15:03:40 +0000 (10:03 -0500)
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

13 files changed:
src/bin/pg_dump/pg_backup_archiver.c
src/bin/pg_dump/pg_dump.c
src/bin/pg_dump/pg_dumpall.c
src/bin/psql/command.c
src/bin/scripts/common.c
src/bin/scripts/createdb.c
src/bin/scripts/createuser.c
src/bin/scripts/dropdb.c
src/bin/scripts/dropuser.c
src/bin/scripts/reindexdb.c
src/bin/scripts/vacuumdb.c
src/fe_utils/string_utils.c
src/include/fe_utils/string_utils.h

index 4c4b301a347df789e9dbf39669bbfb86efcfce65..1040b1b6b6593387fda2ed08dc01cd8b06f9b792 100644 (file)
@@ -2725,6 +2725,7 @@ processEncodingEntry(ArchiveHandle *AH, TocEntry *te)
            fatal("unrecognized encoding \"%s\"",
                  ptr1);
        AH->public.encoding = encoding;
+       setFmtEncoding(encoding);
    }
    else
        fatal("invalid ENCODING item: %s",
index c83e89b87d5d28c660f4782cc11a9f4584c649ef..4c098f20ad3e4a66cf9e262e2212d47c0a9ea891 100644 (file)
@@ -1094,6 +1094,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);
index 1cf8d20829e3ae667003570ba01e7e67dbc8ab5f..1b4c8c0b090e5ae91a5ab667d21e9fde743d4eb4 100644 (file)
@@ -507,6 +507,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";
index 506187cfdd5fff63f80cdcfb9f41a4321854a458..a00d1b86785dd6ad8dc8543643025937aca78775 100644 (file)
@@ -1220,6 +1220,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));
            }
@@ -3606,6 +3607,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));
index d446d7af9fa053a1b1e8565d0c16e3d4d72b8fbf..8b56ba156abe4ca8be7894ed905d4b549d4b91d6 100644 (file)
@@ -430,8 +430,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);
index 91e6e2194bd7700fa963c25153ab17d914cc723a..5e5190d613e7d0a0fb59f679cb018796e2178cba 100644 (file)
@@ -190,6 +190,8 @@ main(int argc, char *argv[])
 
    conn = connectMaintenanceDatabase(&cparams, progname, echo);
 
+   setFmtEncoding(PQclientEncoding(conn));
+
    initPQExpBuffer(&sql);
 
    appendPQExpBuffer(&sql, "CREATE DATABASE %s",
index a15d21f26434243b4b6a5fb044b53b173277558c..6ee5b324a6e6197e0cae5525159281e3ddd8918e 100644 (file)
@@ -266,6 +266,8 @@ main(int argc, char *argv[])
 
    conn = connectMaintenanceDatabase(&cparams, progname, echo);
 
+   setFmtEncoding(PQclientEncoding(conn));
+
    initPQExpBuffer(&sql);
 
    printfPQExpBuffer(&sql, "CREATE ROLE %s", fmtId(newuser));
index ccbf78e91a86437e8ada464648d7ff4dc6a81efa..02ebaedd49438613870ceba600d8c872bf3347d3 100644 (file)
@@ -127,13 +127,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";
@@ -147,6 +140,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);
index ef4fe4cd18da590f7099e730e85a92a0d5b29cb6..ba613dda68c73985c285807b4ea362c08e03c22a 100644 (file)
@@ -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);
index 8e7fa276549346f32b651a7642f74f7a646aaf0c..d7813b18fd3bd1e90aa16c9f13b4c5749f630805 100644 (file)
@@ -532,7 +532,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:
@@ -702,8 +703,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);
index a46fb7133b70a94dda685f97ba03b4350f5a7323..55c24a4db9aaaf732c224493194caf847a1ca410 100644 (file)
@@ -611,8 +611,9 @@ vacuum_one_database(const 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));
index b4bba26934bdbd56469414c4ccc6d690cccdfb89..ec63be545421c339f27662b11b349e33d36f9cc9 100644 (file)
@@ -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
index 5924d3248a160db80c1d5d61684cbacd68159574..ded83b3e6a294cc5ff762fa0a187fc0c2cb4439b 100644 (file)
@@ -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);