From 0444e07dc9c5798b4839fcb27267372cd5583b46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florin=20Cri=C8=99an?= Date: Thu, 8 Jul 2021 21:14:49 +0300 Subject: [PATCH] Fix #7047 Safely handle setlocale (#8735) * fix #7074 Safely handle setlocale `setlocale` returns a pointer to a buffer containing the current locale name. This needs to be copied into a `std::string` or it will be overwritten by the next call. Trying to call `setlocale` with a non-null, invalid pointer can have unpredictable results, such as ``` [ RUN ] StringPrintfTest.Multibyte minkernel\crts\ucrt\src\appcrt\convert\mbstowcs.cpp(246) : Assertion failed: (pwcs == nullptr && sizeInWords == 0) || (pwcs != nullptr && sizeInWords > 0) ``` `setlocale` can also return a `nullptr` if it fails, but we assert against that. * stringprintf_unittest.cc: Replace `new char[n+1]` with `std::array` Prefer safer alternative to naked pointers. This is a follow-up to 1dd313cd0611ac13257c075a977d46c874c42652 --- .../protobuf/stubs/stringprintf_unittest.cc | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/google/protobuf/stubs/stringprintf_unittest.cc b/src/google/protobuf/stubs/stringprintf_unittest.cc index 37172a9d970..63f38bfe1eb 100644 --- a/src/google/protobuf/stubs/stringprintf_unittest.cc +++ b/src/google/protobuf/stubs/stringprintf_unittest.cc @@ -32,6 +32,7 @@ #include +#include #include #include @@ -91,7 +92,9 @@ TEST(StringPrintfTest, Multibyte) { // out of memory while trying to determine destination buffer size. // see b/4194543. - char* old_locale = setlocale(LC_CTYPE, nullptr); + char* old_locale_c = setlocale(LC_CTYPE, nullptr); + ASSERT_TRUE(old_locale_c != nullptr); + std::string old_locale = old_locale_c; // Push locale with multibyte mode setlocale(LC_CTYPE, "en_US.utf8"); @@ -106,24 +109,25 @@ TEST(StringPrintfTest, Multibyte) { // Repeat with longer string, to make sure that the dynamically // allocated path in StringAppendV is handled correctly. - int n = 2048; - char* buf = new char[n+1]; - memset(buf, ' ', n-3); - memcpy(buf + n - 3, kInvalidCodePoint, 4); - value = StringPrintf("%.*s", n, buf); + const size_t n = 2048; + std::array buf; + memset(&buf[0], ' ', n-3); + memcpy(&buf[0] + n - 3, kInvalidCodePoint, 4); + value = StringPrintf("%.*s", n, &buf[0]); // See GRTEv2 vs. GRTEv3 comment above. - EXPECT_TRUE(value.empty() || value == buf); - delete[] buf; + EXPECT_TRUE(value.empty() || value == &buf[0]); - setlocale(LC_CTYPE, old_locale); + setlocale(LC_CTYPE, old_locale.c_str()); } TEST(StringPrintfTest, NoMultibyte) { // No multibyte handling, but the string contains funny chars. - char* old_locale = setlocale(LC_CTYPE, nullptr); + char* old_locale_c = setlocale(LC_CTYPE, nullptr); + ASSERT_TRUE(old_locale_c != nullptr); + std::string old_locale = old_locale_c; setlocale(LC_CTYPE, "POSIX"); std::string value = StringPrintf("%.*s", 3, "\375\067s"); - setlocale(LC_CTYPE, old_locale); + setlocale(LC_CTYPE, old_locale.c_str()); EXPECT_EQ("\375\067s", value); }