Skip to content

Commit 89a5499

Browse files
tglsfdctglsfdc
committed
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.
1 parent b5bd135 commit 89a5499

File tree

8 files changed

+37
-21
lines changed

8 files changed

+37
-21
lines changed

‎src/bin/psql/common.c

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
#include "fe_utils/mbprint.h"
3030

3131

32+
#define PQmblenBounded(s, e) strnlen(s, PQmblen(s, e))
33+
3234
static bool DescribeQuery(const char *query, double *elapsed_msec);
3335
static bool ExecQueryUsingCursor(const char *query, double *elapsed_msec);
3436
static bool command_no_begin(const char *query);
@@ -2001,7 +2003,7 @@ skip_white_space(const char *query)
20012003

20022004
while (*query)
20032005
{
2004-
int mblen = PQmblen(query, pset.encoding);
2006+
int mblen = PQmblenBounded(query, pset.encoding);
20052007

20062008
/*
20072009
* Note: we assume the encoding is a superset of ASCII, so that for
@@ -2038,7 +2040,7 @@ skip_white_space(const char *query)
20382040
query++;
20392041
break;
20402042
}
2041-
query += PQmblen(query, pset.encoding);
2043+
query += PQmblenBounded(query, pset.encoding);
20422044
}
20432045
}
20442046
else if (cnestlevel > 0)
@@ -2073,7 +2075,7 @@ command_no_begin(const char *query)
20732075
*/
20742076
wordlen = 0;
20752077
while (isalpha((unsigned char) query[wordlen]))
2076-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2078+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
20772079

20782080
/*
20792081
* Transaction control commands. These should include every keyword that
@@ -2104,7 +2106,7 @@ command_no_begin(const char *query)
21042106

21052107
wordlen = 0;
21062108
while (isalpha((unsigned char) query[wordlen]))
2107-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2109+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
21082110

21092111
if (wordlen == 11 && pg_strncasecmp(query, "transaction", 11) == 0)
21102112
return true;
@@ -2138,7 +2140,7 @@ command_no_begin(const char *query)
21382140

21392141
wordlen = 0;
21402142
while (isalpha((unsigned char) query[wordlen]))
2141-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2143+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
21422144

21432145
if (wordlen == 8 && pg_strncasecmp(query, "database", 8) == 0)
21442146
return true;
@@ -2154,7 +2156,7 @@ command_no_begin(const char *query)
21542156

21552157
wordlen = 0;
21562158
while (isalpha((unsigned char) query[wordlen]))
2157-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2159+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
21582160
}
21592161

21602162
if (wordlen == 5 && pg_strncasecmp(query, "index", 5) == 0)
@@ -2165,7 +2167,7 @@ command_no_begin(const char *query)
21652167

21662168
wordlen = 0;
21672169
while (isalpha((unsigned char) query[wordlen]))
2168-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2170+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
21692171

21702172
if (wordlen == 12 && pg_strncasecmp(query, "concurrently", 12) == 0)
21712173
return true;
@@ -2182,7 +2184,7 @@ command_no_begin(const char *query)
21822184

21832185
wordlen = 0;
21842186
while (isalpha((unsigned char) query[wordlen]))
2185-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2187+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
21862188

21872189
/* ALTER SYSTEM isn't allowed in xacts */
21882190
if (wordlen == 6 && pg_strncasecmp(query, "system", 6) == 0)
@@ -2205,7 +2207,7 @@ command_no_begin(const char *query)
22052207

22062208
wordlen = 0;
22072209
while (isalpha((unsigned char) query[wordlen]))
2208-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2210+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
22092211

22102212
if (wordlen == 8 && pg_strncasecmp(query, "database", 8) == 0)
22112213
return true;
@@ -2223,7 +2225,7 @@ command_no_begin(const char *query)
22232225

22242226
wordlen = 0;
22252227
while (isalpha((unsigned char) query[wordlen]))
2226-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2228+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
22272229

22282230
if (wordlen == 12 && pg_strncasecmp(query, "concurrently", 12) == 0)
22292231
return true;
@@ -2243,7 +2245,7 @@ command_no_begin(const char *query)
22432245

22442246
wordlen = 0;
22452247
while (isalpha((unsigned char) query[wordlen]))
2246-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2248+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
22472249

22482250
if (wordlen == 3 && pg_strncasecmp(query, "all", 3) == 0)
22492251
return true;
@@ -2279,7 +2281,7 @@ is_select_command(const char *query)
22792281
*/
22802282
wordlen = 0;
22812283
while (isalpha((unsigned char) query[wordlen]))
2282-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2284+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
22832285

22842286
if (wordlen == 6 && pg_strncasecmp(query, "select", 6) == 0)
22852287
return true;

‎src/bin/psql/psqlscanslash.l

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
%{
2828
#include "fe_utils/psqlscan_int.h"
2929

30+
#define PQmblenBounded(s, e) strnlen(s, PQmblen(s, e))
31+
3032
/*
3133
* We must have a typedef YYSTYPE for yylex's first argument, but this lexer
3234
* doesn't presently make use of that argument, so just declare it as int.
@@ -752,7 +754,7 @@ dequote_downcase_identifier(char *str, bool downcase, int encoding)
752754
{
753755
if (downcase && !inquotes)
754756
*cp = pg_tolower((unsigned char) *cp);
755-
cp += PQmblen(cp, encoding);
757+
cp += PQmblenBounded(cp, encoding);
756758
}
757759
}
758760
}

‎src/bin/psql/stringutils.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
#include "common.h"
1313
#include "stringutils.h"
1414

15+
#define PQmblenBounded(s, e) strnlen(s, PQmblen(s, e))
16+
1517

1618
/*
1719
* Replacement for strtok() (a.k.a. poor man's flex)
@@ -143,7 +145,7 @@ strtokx(const char *s,
143145
/* okay, we have a quoted token, now scan for the closer */
144146
char thisquote = *p++;
145147

146-
for (; *p; p += PQmblen(p, encoding))
148+
for (; *p; p += PQmblenBounded(p, encoding))
147149
{
148150
if (*p == escape && p[1] != '\0')
149151
p++; /* process escaped anything */
@@ -262,7 +264,7 @@ strip_quotes(char *source, char quote, char escape, int encoding)
262264
else if (c == escape && src[1] != '\0')
263265
src++; /* process escaped character */
264266

265-
i = PQmblen(src, encoding);
267+
i = PQmblenBounded(src, encoding);
266268
while (i--)
267269
*dst++ = *src++;
268270
}
@@ -322,7 +324,7 @@ quote_if_needed(const char *source, const char *entails_quote,
322324
else if (strchr(entails_quote, c))
323325
need_quotes = true;
324326

325-
i = PQmblen(src, encoding);
327+
i = PQmblenBounded(src, encoding);
326328
while (i--)
327329
*dst++ = *src++;
328330
}

‎src/bin/psql/tab-complete.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ extern char *filename_completion_function();
6060
#define completion_matches rl_completion_matches
6161
#endif
6262

63+
#define PQmblenBounded(s, e) strnlen(s, PQmblen(s, e))
64+
6365
/* word break characters */
6466
#define WORD_BREAKS "\t\n@$><=;|&{() "
6567

@@ -4128,7 +4130,7 @@ _complete_from_query(const char *simple_query,
41284130
while (*pstr)
41294131
{
41304132
char_length++;
4131-
pstr += PQmblen(pstr, pset.encoding);
4133+
pstr += PQmblenBounded(pstr, pset.encoding);
41324134
}
41334135

41344136
/* Free any prior result */

‎src/bin/scripts/common.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
#include "fe_utils/string_utils.h"
2323

2424

25+
#define PQmblenBounded(s, e) strnlen(s, PQmblen(s, e))
26+
2527
static PGcancel *volatile cancelConn = NULL;
2628
bool CancelRequested = false;
2729

@@ -303,7 +305,7 @@ split_table_columns_spec(const char *spec, int encoding,
303305
cp++;
304306
}
305307
else
306-
cp += PQmblen(cp, encoding);
308+
cp += PQmblenBounded(cp, encoding);
307309
}
308310
*table = pg_strdup(spec);
309311
(*table)[cp - spec] = '\0'; /* no strndup */

‎src/fe_utils/print.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3528,6 +3528,9 @@ strlen_max_width(unsigned char *str, int *target_width, int encoding)
35283528
curr_width += char_width;
35293529

35303530
str += PQmblen((char *) str, encoding);
3531+
3532+
if (str > end) /* Don't overrun invalid string */
3533+
str = end;
35313534
}
35323535

35333536
*target_width = curr_width;

‎src/interfaces/libpq/fe-print.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "libpq-fe.h"
3737
#include "libpq-int.h"
3838

39+
#define PQmblenBounded(s, e) strnlen(s, PQmblen(s, e))
3940

4041
static void do_field(const PQprintOpt *po, const PGresult *res,
4142
const int i, const int j, const int fs_len,
@@ -365,7 +366,7 @@ do_field(const PQprintOpt *po, const PGresult *res,
365366
/* Detect whether field contains non-numeric data */
366367
char ch = '0';
367368

368-
for (p = pval; *p; p += PQmblen(p, res->client_encoding))
369+
for (p = pval; *p; p += PQmblenBounded(p, res->client_encoding))
369370
{
370371
ch = *p;
371372
if (!((ch >= '0' && ch <= '9') ||

‎src/interfaces/libpq/fe-protocol3.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141
((id) == 'T' || (id) == 'D' || (id) == 'd' || (id) == 'V' || \
4242
(id) == 'E' || (id) == 'N' || (id) == 'A')
4343

44+
#define PQmblenBounded(s, e) strnlen(s, PQmblen(s, e))
45+
4446

4547
static void handleSyncLoss(PGconn *conn, char id, int msgLength);
4648
static int getRowDescriptions(PGconn *conn, int msgLength);
@@ -1227,7 +1229,7 @@ reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding)
12271229
if (w <= 0)
12281230
w = 1;
12291231
scroffset += w;
1230-
qoffset += pg_encoding_mblen(encoding, &wquery[qoffset]);
1232+
qoffset += PQmblenBounded(&wquery[qoffset], encoding);
12311233
}
12321234
else
12331235
{
@@ -1295,7 +1297,7 @@ reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding)
12951297
* width.
12961298
*/
12971299
scroffset = 0;
1298-
for (; i < msg->len; i += pg_encoding_mblen(encoding, &msg->data[i]))
1300+
for (; i < msg->len; i += PQmblenBounded(&msg->data[i], encoding))
12991301
{
13001302
int w = pg_encoding_dsplen(encoding, &msg->data[i]);
13011303

0 commit comments

Comments
 (0)