Skip to content

Commit ac600c5

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 7cdb976 commit ac600c5

File tree

8 files changed

+37
-21
lines changed

8 files changed

+37
-21
lines changed

‎src/bin/psql/common.c

+14-12
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
#include "fe_utils/mbprint.h"
2929

3030

31+
#define PQmblenBounded(s, e) strnlen(s, PQmblen(s, e))
32+
3133
static bool ExecQueryUsingCursor(const char *query, double *elapsed_msec);
3234
static bool command_no_begin(const char *query);
3335
static bool is_select_command(const char *query);
@@ -1700,7 +1702,7 @@ skip_white_space(const char *query)
17001702

17011703
while (*query)
17021704
{
1703-
int mblen = PQmblen(query, pset.encoding);
1705+
int mblen = PQmblenBounded(query, pset.encoding);
17041706

17051707
/*
17061708
* Note: we assume the encoding is a superset of ASCII, so that for
@@ -1737,7 +1739,7 @@ skip_white_space(const char *query)
17371739
query++;
17381740
break;
17391741
}
1740-
query += PQmblen(query, pset.encoding);
1742+
query += PQmblenBounded(query, pset.encoding);
17411743
}
17421744
}
17431745
else if (cnestlevel > 0)
@@ -1772,7 +1774,7 @@ command_no_begin(const char *query)
17721774
*/
17731775
wordlen = 0;
17741776
while (isalpha((unsigned char) query[wordlen]))
1775-
wordlen += PQmblen(&query[wordlen], pset.encoding);
1777+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
17761778

17771779
/*
17781780
* Transaction control commands. These should include every keyword that
@@ -1803,7 +1805,7 @@ command_no_begin(const char *query)
18031805

18041806
wordlen = 0;
18051807
while (isalpha((unsigned char) query[wordlen]))
1806-
wordlen += PQmblen(&query[wordlen], pset.encoding);
1808+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
18071809

18081810
if (wordlen == 11 && pg_strncasecmp(query, "transaction", 11) == 0)
18091811
return true;
@@ -1837,7 +1839,7 @@ command_no_begin(const char *query)
18371839

18381840
wordlen = 0;
18391841
while (isalpha((unsigned char) query[wordlen]))
1840-
wordlen += PQmblen(&query[wordlen], pset.encoding);
1842+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
18411843

18421844
if (wordlen == 8 && pg_strncasecmp(query, "database", 8) == 0)
18431845
return true;
@@ -1853,7 +1855,7 @@ command_no_begin(const char *query)
18531855

18541856
wordlen = 0;
18551857
while (isalpha((unsigned char) query[wordlen]))
1856-
wordlen += PQmblen(&query[wordlen], pset.encoding);
1858+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
18571859
}
18581860

18591861
if (wordlen == 5 && pg_strncasecmp(query, "index", 5) == 0)
@@ -1864,7 +1866,7 @@ command_no_begin(const char *query)
18641866

18651867
wordlen = 0;
18661868
while (isalpha((unsigned char) query[wordlen]))
1867-
wordlen += PQmblen(&query[wordlen], pset.encoding);
1869+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
18681870

18691871
if (wordlen == 12 && pg_strncasecmp(query, "concurrently", 12) == 0)
18701872
return true;
@@ -1881,7 +1883,7 @@ command_no_begin(const char *query)
18811883

18821884
wordlen = 0;
18831885
while (isalpha((unsigned char) query[wordlen]))
1884-
wordlen += PQmblen(&query[wordlen], pset.encoding);
1886+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
18851887

18861888
/* ALTER SYSTEM isn't allowed in xacts */
18871889
if (wordlen == 6 && pg_strncasecmp(query, "system", 6) == 0)
@@ -1904,7 +1906,7 @@ command_no_begin(const char *query)
19041906

19051907
wordlen = 0;
19061908
while (isalpha((unsigned char) query[wordlen]))
1907-
wordlen += PQmblen(&query[wordlen], pset.encoding);
1909+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
19081910

19091911
if (wordlen == 8 && pg_strncasecmp(query, "database", 8) == 0)
19101912
return true;
@@ -1922,7 +1924,7 @@ command_no_begin(const char *query)
19221924

19231925
wordlen = 0;
19241926
while (isalpha((unsigned char) query[wordlen]))
1925-
wordlen += PQmblen(&query[wordlen], pset.encoding);
1927+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
19261928

19271929
if (wordlen == 12 && pg_strncasecmp(query, "concurrently", 12) == 0)
19281930
return true;
@@ -1942,7 +1944,7 @@ command_no_begin(const char *query)
19421944

19431945
wordlen = 0;
19441946
while (isalpha((unsigned char) query[wordlen]))
1945-
wordlen += PQmblen(&query[wordlen], pset.encoding);
1947+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
19461948

19471949
if (wordlen == 3 && pg_strncasecmp(query, "all", 3) == 0)
19481950
return true;
@@ -1978,7 +1980,7 @@ is_select_command(const char *query)
19781980
*/
19791981
wordlen = 0;
19801982
while (isalpha((unsigned char) query[wordlen]))
1981-
wordlen += PQmblen(&query[wordlen], pset.encoding);
1983+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
19821984

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

‎src/bin/psql/psqlscanslash.l

+3-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
%{
2727
#include "fe_utils/psqlscan_int.h"
2828

29+
#define PQmblenBounded(s, e) strnlen(s, PQmblen(s, e))
30+
2931
/*
3032
* We must have a typedef YYSTYPE for yylex's first argument, but this lexer
3133
* doesn't presently make use of that argument, so just declare it as int.
@@ -681,7 +683,7 @@ dequote_downcase_identifier(char *str, bool downcase, int encoding)
681683
{
682684
if (downcase && !inquotes)
683685
*cp = pg_tolower((unsigned char) *cp);
684-
cp += PQmblen(cp, encoding);
686+
cp += PQmblenBounded(cp, encoding);
685687
}
686688
}
687689
}

‎src/bin/psql/stringutils.c

+5-3
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

+3-1
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ extern char *filename_completion_function();
5757
#define completion_matches rl_completion_matches
5858
#endif
5959

60+
#define PQmblenBounded(s, e) strnlen(s, PQmblen(s, e))
61+
6062
/* word break characters */
6163
#define WORD_BREAKS "\t\n@$><=;|&{() "
6264

@@ -3308,7 +3310,7 @@ _complete_from_query(int is_schema_query, const char *text, int state)
33083310
while (*pstr)
33093311
{
33103312
char_length++;
3311-
pstr += PQmblen(pstr, pset.encoding);
3313+
pstr += PQmblenBounded(pstr, pset.encoding);
33123314
}
33133315

33143316
/* Free any prior result */

‎src/bin/scripts/common.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
#include "fe_utils/connect.h"
2222
#include "fe_utils/string_utils.h"
2323

24+
#define PQmblenBounded(s, e) strnlen(s, PQmblen(s, e))
25+
2426

2527
static PGcancel *volatile cancelConn = NULL;
2628
bool CancelRequested = false;
@@ -304,7 +306,7 @@ split_table_columns_spec(const char *spec, int encoding,
304306
cp++;
305307
}
306308
else
307-
cp += PQmblen(cp, encoding);
309+
cp += PQmblenBounded(cp, encoding);
308310
}
309311
*table = pg_strdup(spec);
310312
(*table)[cp - spec] = '\0'; /* no strndup */

‎src/fe_utils/print.c

+3
Original file line numberDiff line numberDiff line change
@@ -3527,6 +3527,9 @@ strlen_max_width(unsigned char *str, int *target_width, int encoding)
35273527
curr_width += char_width;
35283528

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

35323535
*target_width = curr_width;

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

+2-1
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,
@@ -358,7 +359,7 @@ do_field(const PQprintOpt *po, const PGresult *res,
358359
/* Detect whether field contains non-numeric data */
359360
char ch = '0';
360361

361-
for (p = pval; *p; p += PQmblen(p, res->client_encoding))
362+
for (p = pval; *p; p += PQmblenBounded(p, res->client_encoding))
362363
{
363364
ch = *p;
364365
if (!((ch >= '0' && ch <= '9') ||

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@
4242
((id) == 'T' || (id) == 'D' || (id) == 'd' || (id) == 'V' || \
4343
(id) == 'E' || (id) == 'N' || (id) == 'A')
4444

45+
#define PQmblenBounded(s, e) strnlen(s, PQmblen(s, e))
46+
4547

4648
static void handleSyncLoss(PGconn *conn, char id, int msgLength);
4749
static int getRowDescriptions(PGconn *conn, int msgLength);
@@ -1228,7 +1230,7 @@ reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding)
12281230
if (w <= 0)
12291231
w = 1;
12301232
scroffset += w;
1231-
qoffset += pg_encoding_mblen(encoding, &wquery[qoffset]);
1233+
qoffset += PQmblenBounded(&wquery[qoffset], encoding);
12321234
}
12331235
else
12341236
{
@@ -1296,7 +1298,7 @@ reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding)
12961298
* width.
12971299
*/
12981300
scroffset = 0;
1299-
for (; i < msg->len; i += pg_encoding_mblen(encoding, &msg->data[i]))
1301+
for (; i < msg->len; i += PQmblenBounded(&msg->data[i], encoding))
13001302
{
13011303
int w = pg_encoding_dsplen(encoding, &msg->data[i]);
13021304

0 commit comments

Comments
 (0)