Fix incautious handling of possibly-miscoded strings in client code.

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.

diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index f16eedbc6841303fc812b49b9a5e7215cbcb6a95..c8325f1c064538180f351b26a9323bea230d30f2 100644 (file)
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -30,6 +30,8 @@
 #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);
@@ -1981,7 +1983,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
@@ -2018,7 +2020,7 @@ skip_white_space(const char *query)
                    query++;
                    break;
                }
-               query += PQmblen(query, pset.encoding);
+               query += PQmblenBounded(query, pset.encoding);
            }
        }
        else if (cnestlevel > 0)
@@ -2053,7 +2055,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
@@ -2084,7 +2086,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;
@@ -2118,7 +2120,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;
@@ -2134,7 +2136,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)
@@ -2145,7 +2147,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;
@@ -2162,7 +2164,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)
@@ -2185,7 +2187,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;
@@ -2200,7 +2202,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
@@ -2219,7 +2221,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;
@@ -2239,7 +2241,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;
@@ -2275,7 +2277,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;

diff --git a/src/bin/psql/psqlscanslash.l b/src/bin/psql/psqlscanslash.l
index 613f66d3bf896576e045cfe97105d6af18b23c5b..e3a31fe5a54c75f9ce3fd406edbeffd9a620e692 100644 (file)
--- a/src/bin/psql/psqlscanslash.l
+++ b/src/bin/psql/psqlscanslash.l
@@ -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);
        }
    }
 }

diff --git a/src/bin/psql/stringutils.c b/src/bin/psql/stringutils.c
index 8c8b4c2fbf13ce5a176a6265d3083f2b3cd6a000..f0abcabbcec63215cbbd9a4f926de0d352c0b1ec 100644 (file)
--- a/src/bin/psql/stringutils.c
+++ b/src/bin/psql/stringutils.c
@@ -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++;
    }
@@ -322,7 +324,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++;
    }

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index b3f550297212e5933490a1a83784eef133fe1c59..c998db447ce880a8445e6a4780938eed7136da24 100644 (file)
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -61,6 +61,8 @@ extern char *filename_completion_function();
 #define completion_matches rl_completion_matches
 #endif
 
+#define PQmblenBounded(s, e)  strnlen(s, PQmblen(s, e))
+
 /* word break characters */
 #define WORD_BREAKS        "\t\n@$><=;|&{() "
 
@@ -3969,7 +3971,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 */

diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c
index 7f4b635571df0f178aaa1ddd9d7b485625175b2b..bb52e6b9ffeccb3d348cb8d31ddd087de082b8b9 100644 (file)
--- a/src/bin/scripts/common.c
+++ b/src/bin/scripts/common.c
@@ -23,6 +23,8 @@
 #include "fe_utils/string_utils.h"
 
 
+#define PQmblenBounded(s, e)  strnlen(s, PQmblen(s, e))
+
 static PGcancel *volatile cancelConn = NULL;
 bool       CancelRequested = false;
 
@@ -300,7 +302,7 @@ splitTableColumnsSpec(const char *spec, int encoding,
            cp++;
        }
        else
-           cp += PQmblen(cp, encoding);
+           cp += PQmblenBounded(cp, encoding);
    }
    *table = pg_strdup(spec);
    (*table)[cp - spec] = '\0'; /* no strndup */

diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c
index e41f42ea981708d636bb09aecebec0f94b00854c..d0c9660548d20adcd2a5cd01ca3d6726a93d0677 100644 (file)
--- a/src/fe_utils/print.c
+++ b/src/fe_utils/print.c
@@ -3653,6 +3653,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;

diff --git a/src/interfaces/libpq/fe-print.c b/src/interfaces/libpq/fe-print.c
index 8b403c678b3c627fbcdc979d03298c6eff824c1b..0b6b17e83908f10511dead3acf3c625ca315ba75 100644 (file)
--- a/src/interfaces/libpq/fe-print.c
+++ b/src/interfaces/libpq/fe-print.c
@@ -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') ||

diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 3bc569e1c005244f4a9042ad4c5a919c43f8f53b..c9f88ba1db9ccc2ef628ed85d5ae8f76ecbdd108 100644 (file)
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -41,6 +41,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);
@@ -1245,7 +1247,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
        {
@@ -1313,7 +1315,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]);