.......PS. [ZBX-10828] fixed server/proxy crashes when performing Simple checks with invalid key parameters hidden in user macro

	   [merge ^/branches/3.0 -c r60655]

7 files changed, 88 insertions, 44 deletions

diff --git a/ChangeLog b/ChangeLog
index 7d880b14598..8234af417a3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -29,6 +29,7 @@ Changes for 3.0.4rc1
 New features:
 
 Bug fixes:
+.......PS. [ZBX-10828] fixed server/proxy crashes when performing Simple checks with invalid key parameters hidden in user macro (viktors)
 ..F....... [ZBX-10789] fixed drawing graphs with items that have scheduling intervals (Cemeris)
 ...G...... [ZBX-10852] fixed agent compilation on AIX 5.2, AIX 5.3 (wiper)
 ..F....... [ZBX-10457] fixed copying triggers to groups with multiple hosts or templates (Ivo)
@@ -1428,6 +1429,7 @@ Changes for 2.2.14rc1
 New features:
 
 Bug fixes:
+.......PS. [ZBX-10828] fixed server/proxy crashes when performing Simple checks with invalid key parameters hidden in user macro (viktors)
 .......PS. [ZBX-10819] fixed server/proxy compilation error on Solaris 10 (viktors)
 ........S. [ZBX-10723] forced quoting of item key parameter if macro resolution resulted in unquoted parameter with leading spaces (gleb)
 .......PS. [ZBX-8096] allowed "noSuchName" to be returned for SNMPv2 and SNMPv3 (asaveljevs)
diff --git a/include/common.h b/include/common.h
index fdfb1ea7f5e..7ecc5d9867c 100644
--- a/include/common.h
+++ b/include/common.h
@@ -870,14 +870,18 @@ char	*get_param_dyn(const char *param, int num);
  *                       will be 0; for their parameters - 1 or higher        *
  *      quoted    - [IN] 1 if parameter is quoted; 0 - otherwise              *
  *      cb_data   - [IN] callback function custom data                        *
+ *      param     - [OUT] replaced item key string                            *
  *                                                                            *
- * Return value: NULL if parameter doesn't change; a new string - otherwise   *
+ * Return value: SUCEED - if parameter doesn't change or has been changed     *
+ *                        successfully                                        *
+ *               FAIL   - otherwise                                           *
  *                                                                            *
  * Comments: The new string should be quoted if it contains special           *
  *           characters                                                       *
  *                                                                            *
  ******************************************************************************/
-typedef char	*(*replace_key_param_f)(const char *data, int key_type, int level, int num, int quoted, void *cb_data);
+typedef int	(*replace_key_param_f)(const char *data, int key_type, int level, int num, int quoted, void *cb_data,
+		char **param);
 #define ZBX_KEY_TYPE_ITEM	0
 #define ZBX_KEY_TYPE_OID	1
 int	replace_key_params_dyn(char **data, int key_type, replace_key_param_f cb, void *cb_data, char *error,
diff --git a/include/sysinfo.h b/include/sysinfo.h
index f0338fe120c..0b29feb5a88 100644
--- a/include/sysinfo.h
+++ b/include/sysinfo.h
@@ -210,7 +210,7 @@ void	free_request(AGENT_REQUEST *request);
 int	parse_item_key(const char *itemkey, AGENT_REQUEST *request);
 
 void	unquote_key_param(char *param);
-void	quote_key_param(char **param, int forced);
+int	quote_key_param(char **param, int forced);
 
 int	set_result_type(AGENT_RESULT *result, int value_type, int data_type, char *c);
 void	set_result_meta(AGENT_RESULT *result, zbx_uint64_t lastlogsize, int mtime);
diff --git a/src/libs/zbxcommon/str.c b/src/libs/zbxcommon/str.c
index e942c092ffb..cc2a9846df4 100644
--- a/src/libs/zbxcommon/str.c
+++ b/src/libs/zbxcommon/str.c
@@ -1598,13 +1598,14 @@ char	*get_param_dyn(const char *p, int num)
  * Comments: auxiliary function for replace_key_params_dyn()                  *
  *                                                                            *
  ******************************************************************************/
-static void	replace_key_param(char **data, int key_type, size_t l, size_t *r, int level, int num, int quoted,
+static int	replace_key_param(char **data, int key_type, size_t l, size_t *r, int level, int num, int quoted,
 		replace_key_param_f cb, void *cb_data)
 {
-	char	c = (*data)[*r], *param;
+	char	c = (*data)[*r], *param = NULL;
+	int	ret;
 
 	(*data)[*r] = '\0';
-	param = cb(*data + l, key_type, level, num, quoted, cb_data);
+	ret = cb(*data + l, key_type, level, num, quoted, cb_data, &param);
 	(*data)[*r] = c;
 
 	if (NULL != param)
@@ -1615,6 +1616,8 @@ static void	replace_key_param(char **data, int key_type, size_t l, size_t *r, in
 
 		zbx_free(param);
 	}
+
+	return ret;
 }
 
 /******************************************************************************
@@ -1669,9 +1672,9 @@ int	replace_key_params_dyn(char **data, int key_type, replace_key_param_f cb, vo
 			;
 	}
 
-	replace_key_param(data, key_type, 0, &i, level, num, 0, cb, cb_data);
+	ret = replace_key_param(data, key_type, 0, &i, level, num, 0, cb, cb_data);
 
-	for (; '\0' != (*data)[i]; i++)
+	for (; '\0' != (*data)[i] && FAIL != ret; i++)
 	{
 		if (0 == level)
 		{
@@ -1690,7 +1693,8 @@ int	replace_key_params_dyn(char **data, int key_type, replace_key_param_f cb, vo
 					case ' ':
 						break;
 					case ',':
-						replace_key_param(data, key_type, i, &i, level, num, 0, cb, cb_data);
+						ret = replace_key_param(data, key_type, i, &i, level, num, 0, cb,
+								cb_data);
 						if (1 == level)
 							num++;
 						break;
@@ -1700,7 +1704,8 @@ int	replace_key_params_dyn(char **data, int key_type, replace_key_param_f cb, vo
 							num++;
 						break;
 					case ']':
-						replace_key_param(data, key_type, i, &i, level, num, 0, cb, cb_data);
+						ret = replace_key_param(data, key_type, i, &i, level, num, 0, cb,
+								cb_data);
 						level--;
 						state = ZBX_STATE_END;
 						break;
@@ -1733,7 +1738,7 @@ int	replace_key_params_dyn(char **data, int key_type, replace_key_param_f cb, vo
 			case ZBX_STATE_UNQUOTED:	/* an unquoted parameter */
 				if (']' == (*data)[i] || ',' == (*data)[i])
 				{
-					replace_key_param(data, key_type, l, &i, level, num, 0, cb, cb_data);
+					ret = replace_key_param(data, key_type, l, &i, level, num, 0, cb, cb_data);
 
 					i--;
 					state = ZBX_STATE_END;
@@ -1742,7 +1747,9 @@ int	replace_key_params_dyn(char **data, int key_type, replace_key_param_f cb, vo
 			case ZBX_STATE_QUOTED:	/* a quoted parameter */
 				if ('"' == (*data)[i] && '\\' != (*data)[i - 1])
 				{
-					i++; replace_key_param(data, key_type, l, &i, level, num, 1, cb, cb_data); i--;
+					i++;
+					ret = replace_key_param(data, key_type, l, &i, level, num, 1, cb, cb_data);
+					i--;
 
 					state = ZBX_STATE_END;
 				}
diff --git a/src/libs/zbxdbupgrade/dbupgrade_2010.c b/src/libs/zbxdbupgrade/dbupgrade_2010.c
index f9ec7eed985..bdbd9b63f47 100644
--- a/src/libs/zbxdbupgrade/dbupgrade_2010.c
+++ b/src/libs/zbxdbupgrade/dbupgrade_2010.c
@@ -945,16 +945,29 @@ static int	DBpatch_2010101(void)
 			zbx_strncpy_alloc(&param, &param_alloc, &param_offset, row[1] + 15, key_len - 16);
 
 			if (1 != (nparam = num_param(param)))
-				quote_key_param(&param, 0);
-			quote_key_param(&dsn, 0);
+			{
+				if (FAIL == (ret = quote_key_param(&param, 0)))
+					error_message = zbx_dsprintf(error_message, "unique description"
+							" \"%s\" contains invalid symbols and cannot be quoted", param);
+			}
+			if (FAIL == (ret = quote_key_param(&dsn, 0)))
+			{
+				error_message = zbx_dsprintf(error_message, "data source name"
+						" \"%s\" contains invalid symbols and cannot be quoted", dsn);
+			}
 
-			key_offset = 0;
-			zbx_snprintf_alloc(&key, &key_alloc, &key_offset, "db.odbc.select[%s,%s]", param, dsn);
+			if (SUCCEED == ret)
+			{
+				key_offset = 0;
+				zbx_snprintf_alloc(&key, &key_alloc, &key_offset, "db.odbc.select[%s,%s]", param, dsn);
 
-			zbx_free(param);
+				zbx_free(param);
 
-			if (255 /* ITEM_KEY_LEN */ < zbx_strlen_utf8(key))
-				error_message = zbx_dsprintf(error_message, "key \"%s\" is too long", row[1]);
+				if (255 /* ITEM_KEY_LEN */ < zbx_strlen_utf8(key))
+					error_message = zbx_dsprintf(error_message, "key \"%s\" is too long", row[1]);
+			}
+
+			zbx_free(param);
 		}
 
 		if (NULL == error_message)
@@ -1649,17 +1662,19 @@ static int	DBpatch_2010194(void)
 
 /******************************************************************************
  *                                                                            *
- * Function: replace_key_param                                                *
+ * Function: DBpatch_2010195_replace_key_param_cb                             *
  *                                                                            *
  * Comments: auxiliary function for DBpatch_2010195()                         *
  *                                                                            *
  ******************************************************************************/
-static char	*replace_key_param(const char *data, int key_type, int level, int num, int quoted, void *cb_data)
+static int	DBpatch_2010195_replace_key_param_cb(const char *data, int key_type, int level, int num, int quoted,
+			void *cb_data, char **new_param)
 {
-	char	*param, *new_param;
+	char	*param;
+	int	ret;
 
 	if (1 != level || 4 != num)	/* the fourth parameter on first level should be updated */
-		return NULL;
+		return SUCCEED;
 
 	param = zbx_strdup(NULL, data);
 
@@ -1668,16 +1683,17 @@ static char	*replace_key_param(const char *data, int key_type, int level, int nu
 	if ('\0' == *param)
 	{
 		zbx_free(param);
-		return NULL;
+		return SUCCEED;
 	}
 
-	new_param = zbx_dsprintf(NULL, "^%s$", param);
+	*new_param = zbx_dsprintf(NULL, "^%s$", param);
 
 	zbx_free(param);
 
-	quote_key_param(&new_param, quoted);
+	if (FAIL == (ret = quote_key_param(new_param, quoted)))
+		zbx_free(new_param);
 
-	return new_param;
+	return ret;
 }
 
 static int	DBpatch_2010195(void)
@@ -1693,8 +1709,8 @@ static int	DBpatch_2010195(void)
 	{
 		key = zbx_strdup(key, row[1]);
 
-		if (SUCCEED != replace_key_params_dyn(&key, ZBX_KEY_TYPE_ITEM, replace_key_param, NULL,
-				error, sizeof(error)))
+		if (SUCCEED != replace_key_params_dyn(&key, ZBX_KEY_TYPE_ITEM, DBpatch_2010195_replace_key_param_cb,
+				NULL, error, sizeof(error)))
 		{
 			zabbix_log(LOG_LEVEL_WARNING, "cannot convert item key \"%s\": %s", row[1], error);
 			continue;
diff --git a/src/libs/zbxserver/expression.c b/src/libs/zbxserver/expression.c
index e4601733302..db2a2dc6d91 100644
--- a/src/libs/zbxserver/expression.c
+++ b/src/libs/zbxserver/expression.c
@@ -4701,36 +4701,39 @@ replace_key_param_data_t;
  * Comments: auxiliary function for substitute_key_macros()                   *
  *                                                                            *
  ******************************************************************************/
-static char	*replace_key_param(const char *data, int key_type, int level, int num, int quoted, void *cb_data)
+static int	replace_key_param_cb(const char *data, int key_type, int level, int num, int quoted, void *cb_data,
+			char **param)
 {
 	replace_key_param_data_t	*replace_key_param_data = (replace_key_param_data_t *)cb_data;
 	zbx_uint64_t			*hostid = replace_key_param_data->hostid;
 	DC_ITEM				*dc_item = replace_key_param_data->dc_item;
 	struct zbx_json_parse		*jp_row = replace_key_param_data->jp_row;
-	int				macro_type = replace_key_param_data->macro_type;
-	char				*param;
+	int				macro_type = replace_key_param_data->macro_type, ret = SUCCEED;
 
 	if (ZBX_KEY_TYPE_ITEM == key_type && 0 == level)
-		return NULL;
+		return ret;
 
 	if (NULL == strchr(data, '{'))
-		return NULL;
+		return ret;
 
-	param = zbx_strdup(NULL, data);
+	*param = zbx_strdup(NULL, data);
 
 	if (0 != level)
-		unquote_key_param(param);
+		unquote_key_param(*param);
 
 	if (NULL == jp_row)
 		substitute_simple_macros(NULL, NULL, NULL, NULL, hostid, NULL, dc_item, NULL,
-				&param, macro_type, NULL, 0);
+				param, macro_type, NULL, 0);
 	else
-		substitute_lld_macros(&param, jp_row, ZBX_MACRO_ANY, NULL, NULL, 0);
+		substitute_lld_macros(param, jp_row, ZBX_MACRO_ANY, NULL, NULL, 0);
 
 	if (0 != level)
-		quote_key_param(&param, quoted);
+	{
+		if (FAIL == (ret = quote_key_param(param, quoted)))
+			zbx_free(*param);
+	}
 
-	return param;
+	return ret;
 }
 
 /******************************************************************************
@@ -4744,6 +4747,7 @@ static char	*replace_key_param(const char *data, int key_type, int level, int nu
  *           echo.sh[{$MACRO}]       | a           | echo.sh[a]               *
  *           echo.sh[{$MACRO}]       |  a          | echo.sh[" a"]            *
  *           echo.sh["{$MACRO}"]     | a           | echo.sh["a"]             *
+ *           echo.sh[{$MACRO}]       |  a\         | echo.sh[{$MACRO}]        *
  *           echo.sh[{$MACRO}]       | "a"         | echo.sh["\"a\""]         *
  *           echo.sh["{$MACRO}"]     | "a"         | echo.sh["\"a\""]         *
  *           echo.sh[{$MACRO}]       | a,b         | echo.sh["a,b"]           *
@@ -4778,7 +4782,7 @@ int	substitute_key_macros(char **data, zbx_uint64_t *hostid, DC_ITEM *dc_item, s
 			exit(EXIT_FAILURE);
 	}
 
-	ret = replace_key_params_dyn(data, key_type, replace_key_param, &replace_key_param_data, error, maxerrlen);
+	ret = replace_key_params_dyn(data, key_type, replace_key_param_cb, &replace_key_param_data, error, maxerrlen);
 
 	zabbix_log(LOG_LEVEL_DEBUG, "End of %s():%s data:'%s'", __function_name, zbx_result_string(ret), *data);
 
diff --git a/src/libs/zbxsysinfo/sysinfo.c b/src/libs/zbxsysinfo/sysinfo.c
index 8e3eb2e1ec3..d413c78dab8 100644
--- a/src/libs/zbxsysinfo/sysinfo.c
+++ b/src/libs/zbxsysinfo/sysinfo.c
@@ -1054,20 +1054,29 @@ void	unquote_key_param(char *param)
  *                            0 - do nothing if the paramter does not contain *
  *                                any special characters                      *
  *                                                                            *
+ * Return value: SUCEED - if parameter doesn't contain special characters and *
+ *                        is not forced to be quoted or when parameter is     *
+ *                        enclosed in quotes                                  *
+ *               FAIL   - if parameter ends with backslash                    *
+ *                                                                            *
  ******************************************************************************/
-void	quote_key_param(char **param, int forced)
+int	quote_key_param(char **param, int forced)
 {
 	size_t	sz_src, sz_dst;
 
 	if (0 == forced)
 	{
 		if ('"' != **param && ' ' != **param && NULL == strchr(*param, ',') && NULL == strchr(*param, ']'))
-			return;
+			return SUCCEED;
 	}
 
-	sz_dst = zbx_get_escape_string_len(*param, "\"") + 3;
 	sz_src = strlen(*param);
 
+	if ('\\' == (*param)[sz_src - 1])
+		return FAIL;
+
+	sz_dst = zbx_get_escape_string_len(*param, "\"") + 3;
+
 	*param = zbx_realloc(*param, sz_dst);
 
 	(*param)[--sz_dst] = '\0';
@@ -1080,6 +1089,8 @@ void	quote_key_param(char **param, int forced)
 			(*param)[--sz_dst] = '\\';
 	}
 	(*param)[--sz_dst] = '"';
+
+	return SUCCEED;
 }
 
 #ifdef HAVE_KSTAT_H