[[ Bug 22586 ]] Fix buffer overrun in native split

montegoulding · montegoulding · commit 7b57891b8588 · 2020-10-31T10:43:15.000+11:00
This patch fixes a buffer overrun in the native split codepath where
adding the key length to the ptr used to extract the element could result
in a start pointer higher than the end ptr. Subtracting from the end ptr to
find the length resulted in a very high length due to unsignedness thus causing
invalid access.
diff --git a/docs/notes/bugfix-22586.md b/docs/notes/bugfix-22586.md
@@ -0,0 +1 @@
+# Fix crash in `split` command with multi-char delimiter
diff --git a/libfoundation/src/foundation-string.cpp b/libfoundation/src/foundation-string.cpp
@@ -5263,9 +5263,9 @@ bool MCStringSplitNative(MCStringRef self, MCStringRef p_elem_del, MCStringRef p
 			if (!MCNameCreateWithNativeChars(t_sptr, t_key_end - t_sptr, &t_name))
 				return false;
             
-			if (t_key_end != t_element_end)
+			if (t_key_end <= t_element_end - p_key_del -> char_count)
 				t_key_end += p_key_del -> char_count;
-            
+
 			MCAutoStringRef t_string;
 			if (!MCStringCreateWithNativeChars(t_key_end, t_element_end - t_key_end, &t_string))
 				return false;
diff --git a/tests/lcs/core/array/split.livecodescript b/tests/lcs/core/array/split.livecodescript
@@ -239,3 +239,14 @@ on TestSplitAsSet
    split tResult with empty as set
    TestAssert "split set (native, empty)", tResult is tExpected
 end TestSplitAsSet
+
+on TestBug22586
+	local tVar
+	put "foo" into tVar
+	split tVar by return and ":   "
+	TestAssert "native split by unfound multi-char delimiter does not crash", true
+
+	put "foo" & numToCodePoint(0x2192) into tVar
+	split tVar by return and ":   "
+	TestAssert "unicode split by unfound multi-char delimiter does not crash", true
+end TestBug22586

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+# Fix crash in `split` command with multi-char delimiter