[PATCH 1/2] settings: fix segfault if when settings has no group

[PATCH 1/2] settings: fix segfault if when settings has no group

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1454 bytes --]

A settings file containing a key/value but no group caused
l_settings_load_from_data to segfault. This was due to both parse_key
and parse_value not checking that the group was non-NULL.

Also, the return of parse_key was being checked against false, when
it actually is returning a unsigned int. This was changed to just
check !parse_key.
---
 ell/settings.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/ell/settings.c b/ell/settings.c
index e3e8303..2153218 100644
--- a/ell/settings.c
+++ b/ell/settings.c
@@ -297,6 +297,9 @@ static unsigned int parse_key(struct l_settings *settings, const char *data,
 	}
 
 	group = l_queue_peek_tail(settings->groups);
+	if (!group)
+		return 0;
+
 	pair = l_new(struct setting_data, 1);
 	pair->key = l_strndup(data, end);
 	l_queue_push_head(group->settings, pair);
@@ -312,6 +315,9 @@ static bool parse_value(struct l_settings *settings, const char *data,
 	struct setting_data *pair;
 
 	group = l_queue_peek_tail(settings->groups);
+	if (!group)
+		return false;
+
 	pair = l_queue_pop_head(group->settings);
 
 	if (!l_utf8_validate(data, len, NULL)) {
@@ -347,7 +353,7 @@ static bool parse_keyvalue(struct l_settings *settings, const char *data,
 		return false;
 	}
 
-	if (parse_key(settings, data, equal - data, line) == false)
+	if (!parse_key(settings, data, equal - data, line))
 		return false;
 
 	equal += 1;
-- 
2.17.1

[PATCH 2/2] unit: test-settings: added test with no group

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]

---
 unit/test-settings.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/unit/test-settings.c b/unit/test-settings.c
index c31c379..71f3341 100644
--- a/unit/test-settings.c
+++ b/unit/test-settings.c
@@ -290,6 +290,20 @@ static void test_to_data(const void *test_data)
 	l_settings_free(settings);
 }
 
+static const char *no_group_data = "key_without_group=value\n";
+
+static void test_invalid_data(const void *test_data)
+{
+	struct l_settings *settings;
+	bool r;
+
+	settings = l_settings_new();
+
+	r = l_settings_load_from_data(settings, no_group_data,
+					strlen(no_group_data));
+	assert(r == false);
+}
+
 int main(int argc, char *argv[])
 {
 	l_test_init(&argc, &argv);
@@ -298,6 +312,7 @@ int main(int argc, char *argv[])
 	l_test_add("Load from File", test_load_from_file, NULL);
 	l_test_add("Set Methods", test_set_methods, NULL);
 	l_test_add("Export to Data 1", test_to_data, data2);
+	l_test_add("Invalid Data (no group)", test_invalid_data, NULL);
 
 	return l_test_run();
 }
-- 
2.17.1

Re: [PATCH 2/2] unit: test-settings: added test with no group

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 199 bytes --]

Hi James,

On 02/21/2019 12:22 PM, James Prestwood wrote:
> ---
>   unit/test-settings.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 

Applied, thanks.

Regards,
-Denis

Re: [PATCH 1/2] settings: fix segfault if when settings has no group

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

Hi James,

On 02/21/2019 12:22 PM, James Prestwood wrote:
> A settings file containing a key/value but no group caused
> l_settings_load_from_data to segfault. This was due to both parse_key
> and parse_value not checking that the group was non-NULL.

Whoops :)

I think it might be more clear to fix this in 
l_settings_load_from_data() to not call parse_keyvalue if no group has 
been encountered previously.

> 
> Also, the return of parse_key was being checked against false, when
> it actually is returning a unsigned int. This was changed to just
> check !parse_key.
> ---
>   ell/settings.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 

Regards,
-Denis