diff options
author | Tom Lane | 2021-06-07 18:15:25 +0000 |
---|---|---|
committer | Tom Lane | 2021-06-07 18:15:25 +0000 |
commit | 42f94f56bf9559f0a3cf5f3ffde50655834694ee (patch) | |
tree | a76ebdb52e2e0ece3982e2de079166a621bfbc20 /src/interfaces | |
parent | 68a6d8a87006ba727d9662ec84c7a3d9de402df0 (diff) |
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.
Diffstat (limited to 'src/interfaces')
-rw-r--r-- | src/interfaces/libpq/exports.txt | 1 | ||||
-rw-r--r-- | src/interfaces/libpq/fe-misc.c | 19 | ||||
-rw-r--r-- | src/interfaces/libpq/fe-print.c | 2 | ||||
-rw-r--r-- | src/interfaces/libpq/fe-protocol3.c | 4 | ||||
-rw-r--r-- | src/interfaces/libpq/libpq-fe.h | 3 |
5 files changed, 24 insertions, 5 deletions
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt index a00701f2c5f..9ef99f6de12 100644 --- a/src/interfaces/libpq/exports.txt +++ b/src/interfaces/libpq/exports.txt @@ -184,3 +184,4 @@ PQexitPipelineMode 181 PQpipelineSync 182 PQpipelineStatus 183 PQtraceSetFlags 184 +PQmblenBounded 185 diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index b347d7f8479..9a2a9702934 100644 --- a/src/interfaces/libpq/fe-misc.c +++ b/src/interfaces/libpq/fe-misc.c @@ -1180,8 +1180,13 @@ pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time) */ /* - * returns the byte length of the character beginning at s, using the + * Returns the byte length of the character beginning at s, using the * specified encoding. + * + * 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 PQmblenBounded() instead. */ int PQmblen(const char *s, int encoding) @@ -1190,7 +1195,17 @@ PQmblen(const char *s, int encoding) } /* - * returns the display length of the character beginning at s, using the + * Returns the byte length of the character beginning at s, using the + * specified encoding; but not more than the distance to end of string. + */ +int +PQmblenBounded(const char *s, int encoding) +{ + return strnlen(s, pg_encoding_mblen(encoding, s)); +} + +/* + * Returns the display length of the character beginning at s, using the * specified encoding. */ int diff --git a/src/interfaces/libpq/fe-print.c b/src/interfaces/libpq/fe-print.c index 94219b1825b..fc7d84844e1 100644 --- a/src/interfaces/libpq/fe-print.c +++ b/src/interfaces/libpq/fe-print.c @@ -365,7 +365,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 b45fb7e7059..2e833053487 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -1296,7 +1296,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 { @@ -1364,7 +1364,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]); diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h index 227adde5a5e..845b4c04c9c 100644 --- a/src/interfaces/libpq/libpq-fe.h +++ b/src/interfaces/libpq/libpq-fe.h @@ -625,6 +625,9 @@ extern int PQlibVersion(void); /* Determine length of multibyte encoded char at *s */ extern int PQmblen(const char *s, int encoding); +/* Same, but not more than the distance to the end of string s */ +extern int PQmblenBounded(const char *s, int encoding); + /* Determine display length of multibyte encoded char at *s */ extern int PQdsplen(const char *s, int encoding); |