Fix incautious handling of possibly-miscoded strings in client code.
authorTom Lane
Mon, 7 Jun 2021 18:15:25 +0000 (14:15 -0400)
committerTom Lane
Mon, 7 Jun 2021 18:15:25 +0000 (14:15 -0400)
An incorrectly-encoded multibyte character near the end of a string
could cause various processing loops to run past the string's
terminating NUL, with results ranging from no detectable issue to
a program crash, depending on what happens to be in the following
memory.

This isn't an issue in the server, because we take care to verify
the encoding of strings before doing any interesting processing
on them.  However, that lack of care leaked into client-side code
which shouldn't assume that anyone has validated the encoding of
its input.

Although this is certainly a bug worth fixing, the PG security team
elected not to regard it as a security issue, primarily because
any untrusted text should be sanitized by PQescapeLiteral or
the like before being incorporated into a SQL or psql command.
(If an app fails to do so, the same technique can be used to
cause SQL injection, with probably much more dire consequences
than a mere client-program crash.)  Those functions were already
made proof against this class of problem, cf CVE-2006-2313.

To fix, invent PQmblenBounded() which is like PQmblen() except it
won't return more than the number of bytes remaining in the string.
In HEAD we can make this a new libpq function, as PQmblen() is.
It seems imprudent to change libpq's API in stable branches though,
so in the back branches define PQmblenBounded as a macro in the files
that need it.  (Note that just changing PQmblen's behavior would not
be a good idea; notably, it would completely break the escaping
functions' defense against this exact problem.  So we just want a
version for those callers that don't have any better way of handling
this issue.)

Per private report from houjingyi.  Back-patch to all supported branches.

src/bin/psql/common.c
src/bin/psql/psqlscanslash.l
src/bin/psql/stringutils.c
src/bin/psql/tab-complete.c
src/bin/scripts/common.c
src/common/jsonapi.c
src/common/wchar.c
src/fe_utils/print.c
src/include/mb/pg_wchar.h
src/interfaces/libpq/fe-print.c
src/interfaces/libpq/fe-protocol3.c

index 10314288499c4aa29197fc9c399ed249938e6239..94d5d60f87929546278cfdb579d09b6e8ff309a0 100644 (file)
@@ -29,6 +29,8 @@
 #include "portability/instr_time.h"
 #include "settings.h"
 
+#define PQmblenBounded(s, e)  strnlen(s, PQmblen(s, e))
+
 static bool DescribeQuery(const char *query, double *elapsed_msec);
 static bool ExecQueryUsingCursor(const char *query, double *elapsed_msec);
 static bool command_no_begin(const char *query);
@@ -1842,7 +1844,7 @@ skip_white_space(const char *query)
 
    while (*query)
    {
-       int         mblen = PQmblen(query, pset.encoding);
+       int         mblen = PQmblenBounded(query, pset.encoding);
 
        /*
         * Note: we assume the encoding is a superset of ASCII, so that for
@@ -1879,7 +1881,7 @@ skip_white_space(const char *query)
                    query++;
                    break;
                }
-               query += PQmblen(query, pset.encoding);
+               query += PQmblenBounded(query, pset.encoding);
            }
        }
        else if (cnestlevel > 0)
@@ -1914,7 +1916,7 @@ command_no_begin(const char *query)
     */
    wordlen = 0;
    while (isalpha((unsigned char) query[wordlen]))
-       wordlen += PQmblen(&query[wordlen], pset.encoding);
+       wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
 
    /*
     * Transaction control commands.  These should include every keyword that
@@ -1945,7 +1947,7 @@ command_no_begin(const char *query)
 
        wordlen = 0;
        while (isalpha((unsigned char) query[wordlen]))
-           wordlen += PQmblen(&query[wordlen], pset.encoding);
+           wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
 
        if (wordlen == 11 && pg_strncasecmp(query, "transaction", 11) == 0)
            return true;
@@ -1979,7 +1981,7 @@ command_no_begin(const char *query)
 
        wordlen = 0;
        while (isalpha((unsigned char) query[wordlen]))
-           wordlen += PQmblen(&query[wordlen], pset.encoding);
+           wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
 
        if (wordlen == 8 && pg_strncasecmp(query, "database", 8) == 0)
            return true;
@@ -1995,7 +1997,7 @@ command_no_begin(const char *query)
 
            wordlen = 0;
            while (isalpha((unsigned char) query[wordlen]))
-               wordlen += PQmblen(&query[wordlen], pset.encoding);
+               wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
        }
 
        if (wordlen == 5 && pg_strncasecmp(query, "index", 5) == 0)
@@ -2006,7 +2008,7 @@ command_no_begin(const char *query)
 
            wordlen = 0;
            while (isalpha((unsigned char) query[wordlen]))
-               wordlen += PQmblen(&query[wordlen], pset.encoding);
+               wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
 
            if (wordlen == 12 && pg_strncasecmp(query, "concurrently", 12) == 0)
                return true;
@@ -2023,7 +2025,7 @@ command_no_begin(const char *query)
 
        wordlen = 0;
        while (isalpha((unsigned char) query[wordlen]))
-           wordlen += PQmblen(&query[wordlen], pset.encoding);
+           wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
 
        /* ALTER SYSTEM isn't allowed in xacts */
        if (wordlen == 6 && pg_strncasecmp(query, "system", 6) == 0)
@@ -2046,7 +2048,7 @@ command_no_begin(const char *query)
 
        wordlen = 0;
        while (isalpha((unsigned char) query[wordlen]))
-           wordlen += PQmblen(&query[wordlen], pset.encoding);
+           wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
 
        if (wordlen == 8 && pg_strncasecmp(query, "database", 8) == 0)
            return true;
@@ -2061,7 +2063,7 @@ command_no_begin(const char *query)
            query = skip_white_space(query);
            wordlen = 0;
            while (isalpha((unsigned char) query[wordlen]))
-               wordlen += PQmblen(&query[wordlen], pset.encoding);
+               wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
 
            /*
             * REINDEX [ TABLE | INDEX ] CONCURRENTLY are not allowed in
@@ -2080,7 +2082,7 @@ command_no_begin(const char *query)
 
            wordlen = 0;
            while (isalpha((unsigned char) query[wordlen]))
-               wordlen += PQmblen(&query[wordlen], pset.encoding);
+               wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
 
            if (wordlen == 12 && pg_strncasecmp(query, "concurrently", 12) == 0)
                return true;
@@ -2100,7 +2102,7 @@ command_no_begin(const char *query)
 
        wordlen = 0;
        while (isalpha((unsigned char) query[wordlen]))
-           wordlen += PQmblen(&query[wordlen], pset.encoding);
+           wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
 
        if (wordlen == 3 && pg_strncasecmp(query, "all", 3) == 0)
            return true;
@@ -2136,7 +2138,7 @@ is_select_command(const char *query)
     */
    wordlen = 0;
    while (isalpha((unsigned char) query[wordlen]))
-       wordlen += PQmblen(&query[wordlen], pset.encoding);
+       wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
 
    if (wordlen == 6 && pg_strncasecmp(query, "select", 6) == 0)
        return true;
index 4dff84d6271a81b5282e1dbb630fdf0a437720da..390dc6c5c903fbffc5a0bf3279a3f120e2477275 100644 (file)
@@ -28,6 +28,8 @@
 %{
 #include "fe_utils/psqlscan_int.h"
 
+#define PQmblenBounded(s, e)  strnlen(s, PQmblen(s, e))
+
 /*
  * We must have a typedef YYSTYPE for yylex's first argument, but this lexer
  * doesn't presently make use of that argument, so just declare it as int.
@@ -753,7 +755,7 @@ dequote_downcase_identifier(char *str, bool downcase, int encoding)
        {
            if (downcase && !inquotes)
                *cp = pg_tolower((unsigned char) *cp);
-           cp += PQmblen(cp, encoding);
+           cp += PQmblenBounded(cp, encoding);
        }
    }
 }
index c521749661c30eca40f422ff7488f0bd1f322f55..c96b2fe1965a23a9129c1bcda65690f0321e6c11 100644 (file)
@@ -12,6 +12,8 @@
 #include "common.h"
 #include "stringutils.h"
 
+#define PQmblenBounded(s, e)  strnlen(s, PQmblen(s, e))
+
 
 /*
  * Replacement for strtok() (a.k.a. poor man's flex)
@@ -143,7 +145,7 @@ strtokx(const char *s,
        /* okay, we have a quoted token, now scan for the closer */
        char        thisquote = *p++;
 
-       for (; *p; p += PQmblen(p, encoding))
+       for (; *p; p += PQmblenBounded(p, encoding))
        {
            if (*p == escape && p[1] != '\0')
                p++;            /* process escaped anything */
@@ -262,7 +264,7 @@ strip_quotes(char *source, char quote, char escape, int encoding)
        else if (c == escape && src[1] != '\0')
            src++;              /* process escaped character */
 
-       i = PQmblen(src, encoding);
+       i = PQmblenBounded(src, encoding);
        while (i--)
            *dst++ = *src++;
    }
@@ -324,7 +326,7 @@ quote_if_needed(const char *source, const char *entails_quote,
        else if (strchr(entails_quote, c))
            need_quotes = true;
 
-       i = PQmblen(src, encoding);
+       i = PQmblenBounded(src, encoding);
        while (i--)
            *dst++ = *src++;
    }
index eb018854a5c5594987458f0b303cb9beea56afa1..4f46cd949d936b6871e5fd94124e9ae4997bbd92 100644 (file)
@@ -73,6 +73,8 @@
 #define USE_FILENAME_QUOTING_FUNCTIONS 1
 #endif
 
+#define PQmblenBounded(s, e)  strnlen(s, PQmblen(s, e))
+
 /* word break characters */
 #define WORD_BREAKS        "\t\n@$><=;|&{() "
 
@@ -4140,7 +4142,7 @@ _complete_from_query(const char *simple_query,
        while (*pstr)
        {
            char_length++;
-           pstr += PQmblen(pstr, pset.encoding);
+           pstr += PQmblenBounded(pstr, pset.encoding);
        }
 
        /* Free any prior result */
index afa62e4da73fb98fa9cb9fafbb3992523ab526db..d446d7af9fa053a1b1e8565d0c16e3d4d72b8fbf 100644 (file)
@@ -25,6 +25,8 @@
 
 #define ERRCODE_UNDEFINED_TABLE  "42P01"
 
+#define PQmblenBounded(s, e)  strnlen(s, PQmblen(s, e))
+
 /*
  * Provide strictly harmonized handling of --help and --version
  * options.
@@ -368,7 +370,7 @@ splitTableColumnsSpec(const char *spec, int encoding,
            cp++;
        }
        else
-           cp += PQmblen(cp, encoding);
+           cp += PQmblenBounded(cp, encoding);
    }
    *table = pnstrdup(spec, cp - spec);
    *columns = cp;
index 9326f805366dd7b1bb79469c5fac919ca4bde188..6fe17a3d37882ad1629553b1bfd246ee181ae814 100644 (file)
@@ -738,7 +738,7 @@ json_lex_string(JsonLexContext *lex)
                        ch = (ch * 16) + (*s - 'A') + 10;
                    else
                    {
-                       lex->token_terminator = s + pg_encoding_mblen(lex->input_encoding, s);
+                       lex->token_terminator = s + pg_encoding_mblen_bounded(lex->input_encoding, s);
                        return JSON_UNICODE_ESCAPE_FORMAT;
                    }
                }
@@ -844,7 +844,7 @@ json_lex_string(JsonLexContext *lex)
                    default:
                        /* Not a valid string escape, so signal error. */
                        lex->token_start = s;
-                       lex->token_terminator = s + pg_encoding_mblen(lex->input_encoding, s);
+                       lex->token_terminator = s + pg_encoding_mblen_bounded(lex->input_encoding, s);
                        return JSON_ESCAPING_INVALID;
                }
            }
@@ -858,7 +858,7 @@ json_lex_string(JsonLexContext *lex)
                 * shown it's not a performance win.
                 */
                lex->token_start = s;
-               lex->token_terminator = s + pg_encoding_mblen(lex->input_encoding, s);
+               lex->token_terminator = s + pg_encoding_mblen_bounded(lex->input_encoding, s);
                return JSON_ESCAPING_INVALID;
            }
 
index efaf1c155bbdec453a603596e18b0b84729b97a1..0873480223628e8a979c89e441ea62bc3306a946 100644 (file)
@@ -1549,6 +1549,11 @@ const pg_wchar_tbl pg_wchar_table[] = {
 
 /*
  * Returns the byte length of a multibyte character.
+ *
+ * Caution: when dealing with text that is not certainly valid in the
+ * specified encoding, the result may exceed the actual remaining
+ * string length.  Callers that are not prepared to deal with that
+ * should use pg_encoding_mblen_bounded() instead.
  */
 int
 pg_encoding_mblen(int encoding, const char *mbstr)
@@ -1558,6 +1563,16 @@ pg_encoding_mblen(int encoding, const char *mbstr)
            pg_wchar_table[PG_SQL_ASCII].mblen((const unsigned char *) mbstr));
 }
 
+/*
+ * Returns the byte length of a multibyte character; but not more than
+ * the distance to end of string.
+ */
+int
+pg_encoding_mblen_bounded(int encoding, const char *mbstr)
+{
+   return strnlen(mbstr, pg_encoding_mblen(encoding, mbstr));
+}
+
 /*
  * Returns the display length of a multibyte character.
  */
index f3c176aa555998c5ce5da51b7e94887c9d827b45..966a7721801b24465c515475b1937d501435580e 100644 (file)
@@ -3652,6 +3652,9 @@ strlen_max_width(unsigned char *str, int *target_width, int encoding)
        curr_width += char_width;
 
        str += PQmblen((char *) str, encoding);
+
+       if (str > end)          /* Don't overrun invalid string */
+           str = end;
    }
 
    *target_width = curr_width;
index 494aefc7fab8a9669e3eaa59be4737a6a24a996d..b43454b64cea76a76c16cb344fef9845c0912eb1 100644 (file)
@@ -553,6 +553,7 @@ extern int  pg_valid_server_encoding_id(int encoding);
  * earlier in this file are also available from libpgcommon.
  */
 extern int pg_encoding_mblen(int encoding, const char *mbstr);
+extern int pg_encoding_mblen_bounded(int encoding, const char *mbstr);
 extern int pg_encoding_dsplen(int encoding, const char *mbstr);
 extern int pg_encoding_verifymb(int encoding, const char *mbstr, int len);
 extern int pg_encoding_max_length(int encoding);
index 784eaae48e5b03e2e6b4d1a081c55dda662283f6..61ba4e4ce7b35946bdff39d15c80be1a2ac13642 100644 (file)
@@ -36,6 +36,7 @@
 #include "libpq-fe.h"
 #include "libpq-int.h"
 
+#define PQmblenBounded(s, e)  strnlen(s, PQmblen(s, e))
 
 static void do_field(const PQprintOpt *po, const PGresult *res,
                     const int i, const int j, const int fs_len,
@@ -365,7 +366,7 @@ do_field(const PQprintOpt *po, const PGresult *res,
            /* Detect whether field contains non-numeric data */
            char        ch = '0';
 
-           for (p = pval; *p; p += PQmblen(p, res->client_encoding))
+           for (p = pval; *p; p += PQmblenBounded(p, res->client_encoding))
            {
                ch = *p;
                if (!((ch >= '0' && ch <= '9') ||
index 686299dce91f9ec9b207e99f756d58b5f495c55d..04fd4ba874b0c8a5a6f248439800254ae0710b5d 100644 (file)
@@ -39,6 +39,8 @@
    ((id) == 'T' || (id) == 'D' || (id) == 'd' || (id) == 'V' || \
     (id) == 'E' || (id) == 'N' || (id) == 'A')
 
+#define PQmblenBounded(s, e)  strnlen(s, PQmblen(s, e))
+
 
 static void handleSyncLoss(PGconn *conn, char id, int msgLength);
 static int getRowDescriptions(PGconn *conn, int msgLength);
@@ -1241,7 +1243,7 @@ reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding)
            if (w <= 0)
                w = 1;
            scroffset += w;
-           qoffset += pg_encoding_mblen(encoding, &wquery[qoffset]);
+           qoffset += PQmblenBounded(&wquery[qoffset], encoding);
        }
        else
        {
@@ -1309,7 +1311,7 @@ reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding)
         * width.
         */
        scroffset = 0;
-       for (; i < msg->len; i += pg_encoding_mblen(encoding, &msg->data[i]))
+       for (; i < msg->len; i += PQmblenBounded(&msg->data[i], encoding))
        {
            int         w = pg_encoding_dsplen(encoding, &msg->data[i]);