fix(developer): prevent buffer overrun in u16tok

Relates-to: #11814 See-also: #11894
keymanapp · Jul 2, 2024 · 69d8b30 · 69d8b30
1 parent bcddccb
commit 69d8b30
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 4 deletions.
diff --git a/developer/src/kmcmplib/src/kmx_u16.cpp b/developer/src/kmcmplib/src/kmx_u16.cpp
@@ -243,7 +243,7 @@ KMX_WCHAR * u16tok(KMX_WCHAR *p, const KMX_WCHAR ch,  KMX_WCHAR **ctx) {
   else {
     *ctx = NULL;
   }
-  return p;
+  return *p ? p : NULL;
 }
 
 KMX_WCHAR * u16tok(KMX_WCHAR* p, const KMX_WCHAR* delim, KMX_WCHAR** ctx) {
@@ -259,13 +259,13 @@ KMX_WCHAR * u16tok(KMX_WCHAR* p, const KMX_WCHAR* delim, KMX_WCHAR** ctx) {
 	if (*q) {
 		*q = 0;
 		q++;
-		while (u16chr(delim, *q)) q++;
+		while (*q && u16chr(delim, *q)) q++;
 		*ctx = q;
 	}
 	else {
 		*ctx = NULL;
 	}
-	return p;
+  return *p ? p : NULL;
 }
 
 double u16tof( KMX_WCHAR* str)

diff --git a/developer/src/kmcmplib/tests/gtest-kmx_u16-test.cpp b/developer/src/kmcmplib/tests/gtest-kmx_u16-test.cpp
@@ -0,0 +1,60 @@
+#include <gtest/gtest.h>
+#include "../src/kmx_u16.h"
+
+/*class kmx_u16_Test : public testing::Test {
+    protected:
+    	kmx_u16_Test() {}
+	    ~kmx_u16_Test() override {}
+	    void SetUp() override {}
+	    void TearDown() override {}
+};*/
+
+TEST(kmx_u16_Test, u16tok_char_delim) {
+  // For char delimiter: KMX_WCHAR * u16tok(KMX_WCHAR *p, const KMX_WCHAR ch,  KMX_WCHAR **ctx) ;
+
+  KMX_WCHAR *ctx = nullptr;
+  EXPECT_EQ(nullptr, u16tok(nullptr, ' ', &ctx));
+
+  KMX_WCHAR buffer[128] = u"test a space  and two";
+  EXPECT_TRUE(!u16cmp(u"test", u16tok(buffer, ' ', &ctx)));
+  EXPECT_TRUE(!u16cmp(u"a", u16tok(nullptr, ' ', &ctx)));
+  EXPECT_TRUE(!u16cmp(u"space", u16tok(nullptr, ' ', &ctx)));
+  EXPECT_TRUE(!u16cmp(u"and", u16tok(nullptr, ' ', &ctx)));
+  EXPECT_TRUE(!u16cmp(u"two", u16tok(nullptr, ' ', &ctx)));
+
+  KMX_WCHAR buffer_space[128] = u" ";
+  EXPECT_EQ(nullptr, u16tok(buffer_space, ' ', &ctx));
+  EXPECT_EQ(nullptr, u16tok(nullptr, ' ', &ctx));
+}
+
+TEST(kmx_u16_Test, u16tok_str_delim) {
+  // For string delimiter: KMX_WCHAR * u16tok(KMX_WCHAR* p, const KMX_WCHAR* ch, KMX_WCHAR** ctx) ;
+
+  KMX_WCHAR *ctx = nullptr;
+  EXPECT_EQ(nullptr, u16tok(nullptr, u" ", &ctx));
+
+  KMX_WCHAR buffer[128] = u"test a space  and two";
+  EXPECT_TRUE(!u16cmp(u"test", u16tok(buffer, u" ", &ctx)));
+  EXPECT_TRUE(!u16cmp(u"a", u16tok(nullptr, u" ", &ctx)));
+  EXPECT_TRUE(!u16cmp(u"space", u16tok(nullptr, u" ", &ctx)));
+  EXPECT_TRUE(!u16cmp(u"and", u16tok(nullptr, u" ", &ctx)));
+  EXPECT_TRUE(!u16cmp(u"two", u16tok(nullptr, u" ", &ctx)));
+
+  KMX_WCHAR buffer_space[128] = u" ";
+  EXPECT_EQ(nullptr, u16tok(buffer_space, u" ", &ctx));
+  EXPECT_EQ(nullptr, u16tok(nullptr, u" ", &ctx));
+}
+
+TEST(kmx_u16_Test, u16tok_str_compare_to_strtok) {
+  // Compare behaviour of strtok:
+  char sbuffer[128] = "test a space  and two";
+  EXPECT_TRUE(!strcmp("test", strtok(sbuffer, " ")));
+  EXPECT_TRUE(!strcmp("a", strtok(nullptr, " ")));
+  EXPECT_TRUE(!strcmp("space", strtok(nullptr, " ")));
+  EXPECT_TRUE(!strcmp("and", strtok(nullptr, " ")));
+  EXPECT_TRUE(!strcmp("two", strtok(nullptr, " ")));
+
+  char sbuffer_space[128] = " ";
+  EXPECT_EQ(nullptr, strtok(sbuffer_space, " "));
+  EXPECT_EQ(nullptr, strtok(nullptr, " "));
+}
diff --git a/developer/src/kmcmplib/tests/meson.build b/developer/src/kmcmplib/tests/meson.build
@@ -177,4 +177,15 @@ gtestcompmsgtest = executable('gtest-compmsg-test', 'gtest-compmsg-test.cpp',
     dependencies: [ icuuc_dep, gtest_dep, gmock_dep ],
   )
 
-test('gtest-compmsg-test', gtestcompmsgtest)
+test('gtest-compmsg-test', gtestcompmsgtest)
+
+gtestkmx_u16test = executable('gtest-kmx_u16-test', 'gtest-kmx_u16-test.cpp',
+    cpp_args: defns + flags,
+    include_directories: inc,
+    name_suffix: name_suffix,
+    link_args: links + tests_links,
+    objects: lib.extract_all_objects(),
+    dependencies: [ icuuc_dep, gtest_dep, gmock_dep ],
+  )
+
+test('gtest-kmx_u16-test', gtestkmx_u16test)