string_conversion: fix pointer math

Since target_ptr is of type uint16_t* already, we don't need to scale
the byte count as the language does that for us.  If it were void*, we
would need this code, but it's not.

In practice it's probably not a big deal due to how we preallocated
memory: when converting UTF8->UTF16, we'd reserve the same number of
code units, and UTF8 takes more code units per codepoint than UTF16,
so the UTF16 vector is always oversized.

When converting UTF32->UTF16, we also reserve the same number of
code units, but since one UTF32 code unit could require two UTF16
code units (for U+10000 codepoints and higher), we would probably
corrupt memory in the process.  The APIs in this module don't seem
to take into account that range in general, so for now I'm only
fixing the memory corruption.

Bug: google-breakpad:768
Change-Id: Ibfaea4e866733ff8d99b505e72c500bd40d11a74
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1732888
Reviewed-by: Mark Mentovai <mark@chromium.org>

3 files changed, 67 insertions, 2 deletions

diff --git a/src/common/common.gyp b/src/common/common.gyp
index cd05afd5..fe646b47 100644
--- a/src/common/common.gyp
+++ b/src/common/common.gyp
@@ -229,6 +229,7 @@
         'simple_string_dictionary_unittest.cc',
         'stabs_reader_unittest.cc',
         'stabs_to_module_unittest.cc',
+        'string_conversion_unittest.cc',
         'test_assembler_unittest.cc',
         'tests/auto_tempdir.h',
         'tests/file_utils.cc',
diff --git a/src/common/string_conversion.cc b/src/common/string_conversion.cc
index 040d3e86..11d60a36 100644
--- a/src/common/string_conversion.cc
+++ b/src/common/string_conversion.cc
@@ -46,7 +46,7 @@ void UTF8ToUTF16(const char *in, vector<uint16_t> *out) {
   out->clear();
   out->insert(out->begin(), source_length, 0);
   uint16_t *target_ptr = &(*out)[0];
-  uint16_t *target_end_ptr = target_ptr + out->capacity() * sizeof(uint16_t);
+  uint16_t *target_end_ptr = target_ptr + out->capacity();
   ConversionResult result = ConvertUTF8toUTF16(&source_ptr, source_end_ptr,
                                                &target_ptr, target_end_ptr,
                                                strictConversion);
@@ -90,7 +90,7 @@ void UTF32ToUTF16(const wchar_t *in, vector<uint16_t> *out) {
   out->clear();
   out->insert(out->begin(), source_length, 0);
   uint16_t *target_ptr = &(*out)[0];
-  uint16_t *target_end_ptr = target_ptr + out->capacity() * sizeof(uint16_t);
+  uint16_t *target_end_ptr = target_ptr + out->capacity();
   ConversionResult result = ConvertUTF32toUTF16(&source_ptr, source_end_ptr,
                                                 &target_ptr, target_end_ptr,
                                                 strictConversion);
diff --git a/src/common/string_conversion_unittest.cc b/src/common/string_conversion_unittest.cc
new file mode 100644
index 00000000..e9f9b55d
--- /dev/null
+++ b/src/common/string_conversion_unittest.cc
@@ -0,0 +1,64 @@
+// Copyright (c) 2019, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// string_conversion_unittest.cc: Unit tests for google_breakpad::UTF* helpers.
+
+#include <string>
+#include <vector>
+
+#include "breakpad_googletest_includes.h"
+#include "common/string_conversion.h"
+
+using google_breakpad::UTF8ToUTF16;
+using google_breakpad::UTF8ToUTF16Char;
+using google_breakpad::UTF16ToUTF8;
+using std::vector;
+
+TEST(StringConversionTest, UTF8ToUTF16) {
+  const char in[] = "aßc";
+  vector<uint16_t> out;
+  vector<uint16_t> exp{'a', 0xdf, 'c', 0};
+  UTF8ToUTF16(in, &out);
+  EXPECT_EQ(4u, out.size());
+  EXPECT_EQ(exp, out);
+}
+
+TEST(StringConversionTest, UTF8ToUTF16Char) {
+  const char in[] = "a";
+  uint16_t out[3] = {0xff, 0xff, 0xff};
+  EXPECT_EQ(1, UTF8ToUTF16Char(in, 1, out));
+  EXPECT_EQ('a', out[0]);
+  EXPECT_EQ(0, out[1]);
+  EXPECT_EQ(0xff, out[2]);
+}
+
+TEST(StringConversionTest, UTF16ToUTF8) {
+  vector<uint16_t> in{'a', 0xdf, 'c', 0};
+  EXPECT_EQ("aßc", UTF16ToUTF8(in, false));
+}