[svn-commits] sgriepentrog: trunk r404677 - in /trunk: ./ funcs/func_strings.c

[SVN commits to the Digium repositories]

Author: sgriepentrog
Date: Thu Jan  2 13:38:09 2014
New Revision: 404677

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=404677
Log:
func_strings: use memmove to prevent overlapping memory on strcpy

When calling REPLACE() with an empty replace-char argument, strcpy
is used to overwrite the the matching <find-char>.  However as the
src and dest arguments to strcpy must not overlap, it causes other
parts of the string to be overwritten with adjacent characters and
the result is mangled.  Patch replaces call to strcpy with memmove
and adds a test suite case for REPLACE.

(closes issue ASTERISK-22910)
Reported by: Gareth Palmer
Review: https://reviewboard.asterisk.org/r/3083/
Patches:
    func_strings.patch uploaded by Gareth Palmer (license 5169)
........

Merged revisions 404674 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 404675 from http://svn.asterisk.org/svn/asterisk/branches/11
........

Merged revisions 404676 from http://svn.asterisk.org/svn/asterisk/branches/12

Modified:
    trunk/   (props changed)
    trunk/funcs/func_strings.c

Propchange: trunk/
------------------------------------------------------------------------------
--- branch-12-merged (original)
+++ branch-12-merged Thu Jan  2 13:38:09 2014
@@ -1,1 +1,1 @@
-/branches/12:1-398558,398560-398577,398579-399305,399307-401390,401392-403290,403292-403778,403781-404568,404581,404592,404605,404613,404652,404663
+/branches/12:1-398558,398560-398577,398579-399305,399307-401390,401392-403290,403292-403778,403781-404568,404581,404592,404605,404613,404652,404663,404676

Modified: trunk/funcs/func_strings.c
URL: http://svnview.digium.com/svn/asterisk/trunk/funcs/func_strings.c?view=diff&rev=404677&r1=404676&r2=404677
==============================================================================
--- trunk/funcs/func_strings.c (original)
+++ trunk/funcs/func_strings.c Thu Jan  2 13:38:09 2014
@@ -841,8 +841,7 @@
 		 * directly there */
 		if (strchr(find, *strptr)) {
 			if (ast_strlen_zero(replace)) {
-				/* Remove character */
-				strcpy(strptr, strptr + 1); /* SAFE */
+				memmove(strptr, strptr + 1, strlen(strptr + 1) + 1);
 				strptr--;
 			} else {
 				/* Replace character */
@@ -1726,6 +1725,76 @@
 	return res;
 }
 
+AST_TEST_DEFINE(test_REPLACE)
+{
+	int i, res = AST_TEST_PASS;
+	struct ast_channel *chan;
+	struct ast_str *str;
+	char expression[256];
+	struct {
+		const char *test_string;
+		const char *find_chars;
+		const char *replace_char;
+		const char *expected;
+	} test_args[] = {
+		{"abc,def", "\\,", "-", "abc-def"},
+		{"abc,abc", "bc",  "a", "aaa,aaa"},
+		{"abc,def", "x",   "?", "abc,def"},
+		{"abc,def", "\\,", "",  "abcdef"}
+	};
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = "func_REPLACE_test";
+		info->category = "/funcs/func_strings/";
+		info->summary = "Test REPLACE function";
+		info->description = "Verify REPLACE behavior";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	if (!(chan = ast_dummy_channel_alloc())) {
+		ast_test_status_update(test, "Unable to allocate dummy channel\n");
+		return AST_TEST_FAIL;
+	}
+
+	if (!(str = ast_str_create(16))) {
+		ast_test_status_update(test, "Unable to allocate dynamic string buffer\n");
+		ast_channel_release(chan);
+		return AST_TEST_FAIL;
+	}
+
+	for (i = 0; i < ARRAY_LEN(test_args); i++) {
+		struct ast_var_t *var = ast_var_assign("TEST_STRING", test_args[i].test_string);
+		if (!var) {
+			ast_test_status_update(test, "Out of memory\n");
+			res = AST_TEST_FAIL;
+			break;
+		}
+
+		AST_LIST_INSERT_HEAD(ast_channel_varshead(chan), var, entries);
+
+		snprintf(expression, sizeof(expression), "${REPLACE(%s,%s,%s)}", var->name, test_args[i].find_chars, test_args[i].replace_char);
+		ast_str_substitute_variables(&str, 0, chan, expression);
+
+		AST_LIST_REMOVE(ast_channel_varshead(chan), var, entries);
+		ast_var_delete(var);
+
+		if (strcasecmp(ast_str_buffer(str), test_args[i].expected)) {
+			ast_test_status_update(test, "Evaluation of '%s' returned '%s' instead of the expected value '%s'\n",
+				expression, ast_str_buffer(str), test_args[i].expected);
+			res = AST_TEST_FAIL;
+			break;
+		}
+	}
+
+	ast_free(str);
+	ast_channel_release(chan);
+
+	return res;
+}
+
 AST_TEST_DEFINE(test_FILTER)
 {
 	int i, res = AST_TEST_PASS;
@@ -1843,6 +1912,7 @@
 	int res = 0;
 
 	AST_TEST_UNREGISTER(test_FIELDNUM);
+	AST_TEST_UNREGISTER(test_REPLACE);
 	AST_TEST_UNREGISTER(test_FILTER);
 	AST_TEST_UNREGISTER(test_STRREPLACE);
 	res |= ast_custom_function_unregister(&fieldqty_function);
@@ -1879,6 +1949,7 @@
 	int res = 0;
 
 	AST_TEST_REGISTER(test_FIELDNUM);
+	AST_TEST_REGISTER(test_REPLACE);
 	AST_TEST_REGISTER(test_FILTER);
 	AST_TEST_REGISTER(test_STRREPLACE);
 	res |= ast_custom_function_register(&fieldqty_function);