Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit fc896f4

Browse files
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 89f49cc commit fc896f4

File tree

8 files changed

+38
-22
lines changed

8 files changed

+38
-22
lines changed

src/bin/psql/common.c

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
#include "settings.h"
3131

3232

33+
#define PQmblenBounded(s, e) strnlen(s, PQmblen(s, e))
34+
3335
static bool DescribeQuery(const char *query, double *elapsed_msec);
3436
static bool ExecQueryUsingCursor(const char *query, double *elapsed_msec);
3537
static bool command_no_begin(const char *query);
@@ -1981,7 +1983,7 @@ skip_white_space(const char *query)
19811983

19821984
while (*query)
19831985
{
1984-
int mblen = PQmblen(query, pset.encoding);
1986+
int mblen = PQmblenBounded(query, pset.encoding);
19851987

19861988
/*
19871989
* Note: we assume the encoding is a superset of ASCII, so that for
@@ -2018,7 +2020,7 @@ skip_white_space(const char *query)
20182020
query++;
20192021
break;
20202022
}
2021-
query += PQmblen(query, pset.encoding);
2023+
query += PQmblenBounded(query, pset.encoding);
20222024
}
20232025
}
20242026
else if (cnestlevel > 0)
@@ -2053,7 +2055,7 @@ command_no_begin(const char *query)
20532055
*/
20542056
wordlen = 0;
20552057
while (isalpha((unsigned char) query[wordlen]))
2056-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2058+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
20572059

20582060
/*
20592061
* Transaction control commands. These should include every keyword that
@@ -2084,7 +2086,7 @@ command_no_begin(const char *query)
20842086

20852087
wordlen = 0;
20862088
while (isalpha((unsigned char) query[wordlen]))
2087-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2089+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
20882090

20892091
if (wordlen == 11 && pg_strncasecmp(query, "transaction", 11) == 0)
20902092
return true;
@@ -2118,7 +2120,7 @@ command_no_begin(const char *query)
21182120

21192121
wordlen = 0;
21202122
while (isalpha((unsigned char) query[wordlen]))
2121-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2123+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
21222124

21232125
if (wordlen == 8 && pg_strncasecmp(query, "database", 8) == 0)
21242126
return true;
@@ -2134,7 +2136,7 @@ command_no_begin(const char *query)
21342136

21352137
wordlen = 0;
21362138
while (isalpha((unsigned char) query[wordlen]))
2137-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2139+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
21382140
}
21392141

21402142
if (wordlen == 5 && pg_strncasecmp(query, "index", 5) == 0)
@@ -2145,7 +2147,7 @@ command_no_begin(const char *query)
21452147

21462148
wordlen = 0;
21472149
while (isalpha((unsigned char) query[wordlen]))
2148-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2150+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
21492151

21502152
if (wordlen == 12 && pg_strncasecmp(query, "concurrently", 12) == 0)
21512153
return true;
@@ -2162,7 +2164,7 @@ command_no_begin(const char *query)
21622164

21632165
wordlen = 0;
21642166
while (isalpha((unsigned char) query[wordlen]))
2165-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2167+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
21662168

21672169
/* ALTER SYSTEM isn't allowed in xacts */
21682170
if (wordlen == 6 && pg_strncasecmp(query, "system", 6) == 0)
@@ -2185,7 +2187,7 @@ command_no_begin(const char *query)
21852187

21862188
wordlen = 0;
21872189
while (isalpha((unsigned char) query[wordlen]))
2188-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2190+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
21892191

21902192
if (wordlen == 8 && pg_strncasecmp(query, "database", 8) == 0)
21912193
return true;
@@ -2200,7 +2202,7 @@ command_no_begin(const char *query)
22002202
query = skip_white_space(query);
22012203
wordlen = 0;
22022204
while (isalpha((unsigned char) query[wordlen]))
2203-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2205+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
22042206

22052207
/*
22062208
* REINDEX [ TABLE | INDEX ] CONCURRENTLY are not allowed in
@@ -2219,7 +2221,7 @@ command_no_begin(const char *query)
22192221

22202222
wordlen = 0;
22212223
while (isalpha((unsigned char) query[wordlen]))
2222-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2224+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
22232225

22242226
if (wordlen == 12 && pg_strncasecmp(query, "concurrently", 12) == 0)
22252227
return true;
@@ -2239,7 +2241,7 @@ command_no_begin(const char *query)
22392241

22402242
wordlen = 0;
22412243
while (isalpha((unsigned char) query[wordlen]))
2242-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2244+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
22432245

22442246
if (wordlen == 3 && pg_strncasecmp(query, "all", 3) == 0)
22452247
return true;
@@ -2275,7 +2277,7 @@ is_select_command(const char *query)
22752277
*/
22762278
wordlen = 0;
22772279
while (isalpha((unsigned char) query[wordlen]))
2278-
wordlen += PQmblen(&query[wordlen], pset.encoding);
2280+
wordlen += PQmblenBounded(&query[wordlen], pset.encoding);
22792281

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

src/bin/psql/psqlscanslash.l

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

31+
#define PQmblenBounded(s, e) strnlen(s, PQmblen(s, e))
32+
3133
/*
3234
* We must have a typedef YYSTYPE for yylex's first argument, but this lexer
3335
* 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)
753755
{
754756
if (downcase && !inquotes)
755757
*cp = pg_tolower((unsigned char) *cp);
756-
cp += PQmblen(cp, encoding);
758+
cp += PQmblenBounded(cp, encoding);
757759
}
758760
}
759761
}

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
@@ -61,6 +61,8 @@ extern char *filename_completion_function();
6161
#define completion_matches rl_completion_matches
6262
#endif
6363

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

@@ -3969,7 +3971,7 @@ _complete_from_query(const char *simple_query,
39693971
while (*pstr)
39703972
{
39713973
char_length++;
3972-
pstr += PQmblen(pstr, pset.encoding);
3974+
pstr += PQmblenBounded(pstr, pset.encoding);
39733975
}
39743976

39753977
/* Free any prior result */

src/bin/scripts/common.c

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

2525

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

@@ -300,7 +302,7 @@ splitTableColumnsSpec(const char *spec, int encoding,
300302
cp++;
301303
}
302304
else
303-
cp += PQmblen(cp, encoding);
305+
cp += PQmblenBounded(cp, encoding);
304306
}
305307
*table = pg_strdup(spec);
306308
(*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
@@ -3653,6 +3653,9 @@ strlen_max_width(unsigned char *str, int *target_width, int encoding)
36533653
curr_width += char_width;
36543654

36553655
str += PQmblen((char *) str, encoding);
3656+
3657+
if (str > end) /* Don't overrun invalid string */
3658+
str = end;
36563659
}
36573660

36583661
*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);
@@ -1245,7 +1247,7 @@ reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding)
12451247
if (w <= 0)
12461248
w = 1;
12471249
scroffset += w;
1248-
qoffset += pg_encoding_mblen(encoding, &wquery[qoffset]);
1250+
qoffset += PQmblenBounded(&wquery[qoffset], encoding);
12491251
}
12501252
else
12511253
{
@@ -1313,7 +1315,7 @@ reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding)
13131315
* width.
13141316
*/
13151317
scroffset = 0;
1316-
for (; i < msg->len; i += pg_encoding_mblen(encoding, &msg->data[i]))
1318+
for (; i < msg->len; i += PQmblenBounded(&msg->data[i], encoding))
13171319
{
13181320
int w = pg_encoding_dsplen(encoding, &msg->data[i]);
13191321

0 commit comments

Comments
 (0)