From b6159202c99d4021fb078cede90b26f94883143d Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 23 Jun 2017 12:22:06 -0400 Subject: Fix memory leakage in ICU encoding conversion, and other code review. Callers of icu_to_uchar() neglected to pfree the result string when done with it. This results in catastrophic memory leaks in varstr_cmp(), because of our prevailing assumption that btree comparison functions don't leak memory. For safety, make all the call sites clean up leaks, though I suspect that we could get away without it in formatting.c. I audited callers of icu_from_uchar() as well, but found no places that seemed to have a comparable issue. Add function API specifications for icu_to_uchar() and icu_from_uchar(); the lack of any thought-through specification is perhaps not unrelated to the existence of this bug in the first place. Fix icu_to_uchar() to guarantee a nul-terminated result; although no existing caller appears to care, the fact that it would have been nul-terminated except in extreme corner cases seems ideally designed to bite someone on the rear someday. Fix ucnv_fromUChars() destCapacity argument --- in the worst case, that could perhaps have led to a non-nul-terminated result, too. Fix icu_from_uchar() to have a more reasonable definition of the function result --- no callers are actually paying attention, so this isn't a live bug, but it's certainly sloppily designed. Const-ify icu_from_uchar()'s input string for consistency. That is not the end of what needs to be done to these functions, but it's as much as I have the patience for right now. Discussion: https://postgr.es/m/1955.1498181798@sss.pgh.pa.us --- src/backend/utils/adt/formatting.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/backend/utils/adt/formatting.c') diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index 31f04ff5ed5..46f45f66541 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -1561,6 +1561,7 @@ str_tolower(const char *buff, size_t nbytes, Oid collid) len_conv = icu_convert_case(u_strToLower, mylocale, &buff_conv, buff_uchar, len_uchar); icu_from_uchar(&result, buff_conv, len_conv); + pfree(buff_uchar); } else #endif @@ -1684,6 +1685,7 @@ str_toupper(const char *buff, size_t nbytes, Oid collid) len_conv = icu_convert_case(u_strToUpper, mylocale, &buff_conv, buff_uchar, len_uchar); icu_from_uchar(&result, buff_conv, len_conv); + pfree(buff_uchar); } else #endif @@ -1808,6 +1810,7 @@ str_initcap(const char *buff, size_t nbytes, Oid collid) len_conv = icu_convert_case(u_strToTitle_default_BI, mylocale, &buff_conv, buff_uchar, len_uchar); icu_from_uchar(&result, buff_conv, len_conv); + pfree(buff_uchar); } else #endif -- cgit v1.2.3