[Asterisk-code-review] res_geolocation: Fix segfault when there's an empty element (asterisk[18])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/19238 )

Change subject: res_geolocation: Fix segfault when there's an empty element
......................................................................

res_geolocation: Fix segfault when there's an empty element

Fixed a segfault caused by var_list_from_loc_info() encountering
an empty location info element.

Fixed an issue in ast_strsep() where a value with only whitespace
wasn't being preserved.

Fixed an issue in ast_variable_list_from_quoted_string() where
an empty value was considered a failure.

ASTERISK-30215
Reported by: Dan Cropp

Change-Id: Ieca64e061a6d9298f0196c694b60d986ef82613a
---
M include/asterisk/strings.h
M main/config.c
M main/utils.c
M res/res_geolocation/geoloc_eprofile.c
M tests/test_config.c
5 files changed, 44 insertions(+), 13 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  Benjamin Keith Ford: Looks good to me, but someone else must approve
  Michael Bradeen: Looks good to me, approved
  Friendly Automation: Approved for Submit




diff --git a/include/asterisk/strings.h b/include/asterisk/strings.h
index d0a4cbb..f8ad414 100644
--- a/include/asterisk/strings.h
+++ b/include/asterisk/strings.h
@@ -265,6 +265,7 @@
   \param sep A single character delimiter.
   \param flags Controls post-processing of the result.
   AST_STRSEP_TRIM trims all leading and trailing whitespace from the result.
+  If the result containes only whitespace, it'll be passed through unchanged.
   AST_STRSEP_STRIP does a trim then strips the outermost quotes.  You may want
   to trim again after the strip.  Just OR both the TRIM and STRIP flags.
   AST_STRSEP_UNESCAPE unescapes '\' sequences.
diff --git a/main/config.c b/main/config.c
index 0e27d76..1074407 100644
--- a/main/config.c
+++ b/main/config.c
@@ -646,15 +646,16 @@
 struct ast_variable *ast_variable_list_append_hint(struct ast_variable **head, struct ast_variable *search_hint, struct ast_variable *newvar)
 {
 	struct ast_variable *curr;
+	struct ast_variable *sh = search_hint;
 	ast_assert(head != NULL);
 
 	if (!*head) {
 		*head = newvar;
 	} else {
-		if (search_hint == NULL) {
-			search_hint = *head;
+		if (sh == NULL) {
+			sh = *head;
 		}
-		for (curr = search_hint; curr->next; curr = curr->next);
+		for (curr = sh; curr->next; curr = curr->next);
 		curr->next = newvar;
 	}
 
@@ -752,12 +753,8 @@
 		}
 
 		item_value = ast_strsep_quoted(&item, nv_sep, quote, AST_STRSEP_ALL);
-		if (!item_value) {
-			ast_variables_destroy(new_list);
-			return NULL;
-		}
 
-		new_var = ast_variable_new(item_name, item_value, "");
+		new_var = ast_variable_new(item_name, item_value ?: "", "");
 		if (!new_var) {
 			ast_variables_destroy(new_list);
 			return NULL;
diff --git a/main/utils.c b/main/utils.c
index 3ab6dc1..6111b86 100644
--- a/main/utils.c
+++ b/main/utils.c
@@ -1849,7 +1849,10 @@
 	}
 
 	if (flags & AST_STRSEP_TRIM) {
-		st = ast_strip(st);
+		char *trimmed = ast_strip(st);
+		if (!ast_strlen_zero(trimmed)) {
+			st = trimmed;
+		}
 	}
 
 	if (flags & AST_STRSEP_UNESCAPE) {
@@ -1910,7 +1913,10 @@
 	}
 
 	if (flags & AST_STRSEP_TRIM) {
-		st = ast_strip(st);
+		char *trimmed = ast_strip(st);
+		if (!ast_strlen_zero(trimmed)) {
+			st = trimmed;
+		}
 	}
 
 	if (flags & AST_STRSEP_UNESCAPE) {
diff --git a/res/res_geolocation/geoloc_eprofile.c b/res/res_geolocation/geoloc_eprofile.c
index 864dd23..09a06c6 100644
--- a/res/res_geolocation/geoloc_eprofile.c
+++ b/res/res_geolocation/geoloc_eprofile.c
@@ -498,6 +498,7 @@
 	enum ast_geoloc_format format, const char *ref_str)
 {
 	struct ast_variable *list = NULL;
+	struct ast_variable *locinfo_list = NULL;
 	struct ast_xml_node *container;
 	struct ast_variable *var = NULL;
 	const char *attr;
@@ -531,7 +532,12 @@
 		ast_variable_list_append(&list, var);
 	}
 
-	ast_variable_list_append(&list, var_list_from_node(container, ref_str));
+	locinfo_list = var_list_from_node(container, ref_str);
+	if (locinfo_list == NULL) {
+		ast_log(LOG_WARNING, "%s: There were no elements in the location info\n", ref_str);
+		SCOPE_EXIT_RTN_VALUE(list, "%s: There were no elements in the location info\n", ref_str);
+	}
+	ast_variable_list_append(&list, locinfo_list);
 
 	if (TRACE_ATLEAST(5)) {
 		struct ast_str *buf = NULL;
diff --git a/tests/test_config.c b/tests/test_config.c
index 166879a..1d44e5d 100644
--- a/tests/test_config.c
+++ b/tests/test_config.c
@@ -1961,13 +1961,13 @@
 		break;
 	}
 
-	parse_string = "abc = 'def', ghi = 'j,kl', mno='pq=r', stu = 'vwx=\"yz\", ABC = \"DEF\"'";
+	parse_string = "000= '', 111=, 222 = , 333 = ' ', abc = 'def', ghi = 'j,kl', mno='pq=r', stu = 'vwx=\"yz\", ABC = \"DEF\"'";
 	list = ast_variable_list_from_quoted_string(parse_string, ",", "=", "'");
 	ast_test_validate(test, list != NULL);
 	str = ast_variable_list_join(list, "|", "^", "@", NULL);
 
 	ast_test_validate(test,
-		strcmp(ast_str_buffer(str), "abc^@def@|ghi^@j,kl@|mno^@pq=r@|stu^@vwx=\"yz\", ABC = \"DEF\"@") == 0);
+		strcmp(ast_str_buffer(str), "000^@@|111^@@|222^@@|333^@ @|abc^@def@|ghi^@j,kl@|mno^@pq=r@|stu^@vwx=\"yz\", ABC = \"DEF\"@") == 0);
 
 	return AST_TEST_PASS;
 }

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/19238
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: Ieca64e061a6d9298f0196c694b60d986ef82613a
Gerrit-Change-Number: 19238
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Michael Bradeen <mbradeen at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220914/d6518227/attachment.html>