On 6/23/17 12:31, Tom Lane wrote:
> icu_to_uchar() and icu_from_uchar(), and perhaps other places, are
> touchingly naive about integer overflow hazards in buffer size
> calculations. I call particular attention to this bit in
> icu_from_uchar():
>
> len_result = UCNV_GET_MAX_BYTES_FOR_STRING(len_uchar,
> ucnv_getMaxCharSize(icu_converter));
>
> The ICU man pages say that that macro is defined as
>
> #define UCNV_GET_MAX_BYTES_FOR_STRING(length, maxCharSize)
> (((int32_t)(length)+10)*(int32_t)(maxCharSize))
>
> which means that getting this to overflow (resulting in
> probably-exploitable memory overruns) would be about as hard as taking
> candy from a baby.
Here is a patch that should address this.
(I don't think the overruns were exploitable. You'd just get a buffer
overflow error from the ucnv_* function.)
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From c909db0741cb7cdf0b6249e063c7ad78cbf93819 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Sat, 24 Jun 2017 09:39:24 -0400
Subject: [PATCH] Refine memory allocation in ICU conversions
The simple calculations done to estimate the size of the output buffers
for ucnv_fromUChars() and ucnv_toUChars() could overflow int32_t for
large strings. To avoid that, go the long way and run the function
first without an output buffer to get the correct output buffer size
requirement.
---
src/backend/utils/adt/pg_locale.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/adt/pg_locale.c
b/src/backend/utils/adt/pg_locale.c
index 0f5ec954c3..5478e39ea7 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1506,14 +1506,22 @@ icu_to_uchar(UChar **buff_uchar, const char *buff,
size_t nbytes)
init_icu_converter();
- len_uchar = 2 * nbytes + 1; /* max length per docs */
- *buff_uchar = palloc(len_uchar * sizeof(**buff_uchar));
status = U_ZERO_ERROR;
- len_uchar = ucnv_toUChars(icu_converter, *buff_uchar, len_uchar,
+ len_uchar = ucnv_toUChars(icu_converter, NULL, 0,
+ buff, nbytes,
&status);
+ if (U_FAILURE(status) && status != U_BUFFER_OVERFLOW_ERROR)
+ ereport(ERROR,
+ (errmsg("ucnv_toUChars failed: %s",
u_errorName(status))));
+
+ *buff_uchar = palloc((len_uchar + 1) * sizeof(**buff_uchar));
+
+ status = U_ZERO_ERROR;
+ len_uchar = ucnv_toUChars(icu_converter, *buff_uchar, len_uchar + 1,
buff, nbytes,
&status);
if (U_FAILURE(status))
ereport(ERROR,
(errmsg("ucnv_toUChars failed: %s",
u_errorName(status))));
+
return len_uchar;
}
@@ -1536,14 +1544,22 @@ icu_from_uchar(char **result, const UChar *buff_uchar,
int32_t len_uchar)
init_icu_converter();
- len_result = UCNV_GET_MAX_BYTES_FOR_STRING(len_uchar,
ucnv_getMaxCharSize(icu_converter));
+ status = U_ZERO_ERROR;
+ len_result = ucnv_fromUChars(icu_converter, NULL, 0,
+ buff_uchar,
len_uchar, &status);
+ if (U_FAILURE(status) && status != U_BUFFER_OVERFLOW_ERROR)
+ ereport(ERROR,
+ (errmsg("ucnv_fromUChars failed: %s",
u_errorName(status))));
+
*result = palloc(len_result + 1);
+
status = U_ZERO_ERROR;
len_result = ucnv_fromUChars(icu_converter, *result, len_result + 1,
buff_uchar,
len_uchar, &status);
if (U_FAILURE(status))
ereport(ERROR,
(errmsg("ucnv_fromUChars failed: %s",
u_errorName(status))));
+
return len_result;
}
#endif /* USE_ICU */
--
2.13.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
