[PATCH 1/8] client: Treat invalid DBus reply same as an error

[PATCH 1/8] client: Treat invalid DBus reply same as an error

[Tim Kourt]

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

---
 client/dbus-proxy.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/client/dbus-proxy.c b/client/dbus-proxy.c
index 3e7a7f8e..1bd90aff 100644
--- a/client/dbus-proxy.c
+++ b/client/dbus-proxy.c
@@ -699,7 +699,16 @@ static void get_managed_objects_callback(struct l_dbus_message *message,
 		return;
 	}
 
-	l_dbus_message_get_arguments(message, "a{oa{sa{sv}}}", &objects);
+	if (!l_dbus_message_get_arguments(message, "a{oa{sa{sv}}}", &objects)) {
+		l_error("Failed to parse IWD dbus objects, quitting...\n");
+
+		if (!command_is_interactive_mode())
+			command_set_exit_status(EXIT_FAILURE);
+
+		l_main_quit();
+
+		return;
+	}
 
 	while (l_dbus_message_iter_next_entry(&objects, &path, &object))
 		proxy_interface_create(path, &object);
-- 
2.13.6

[PATCH 2/8] client: Ignore invalid notifications

[Tim Kourt]

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

---
 client/dbus-proxy.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/client/dbus-proxy.c b/client/dbus-proxy.c
index 1bd90aff..26c0511d 100644
--- a/client/dbus-proxy.c
+++ b/client/dbus-proxy.c
@@ -651,7 +651,9 @@ static void interfaces_added_callback(struct l_dbus_message *message,
 	if (dbus_message_has_error(message))
 		return;
 
-	l_dbus_message_get_arguments(message, "oa{sa{sv}}", &path, &object);
+	if (!l_dbus_message_get_arguments(message, "oa{sa{sv}}", &path,
+								&object))
+		return;
 
 	proxy_interface_create(path, &object);
 }
@@ -667,7 +669,8 @@ static void interfaces_removed_callback(struct l_dbus_message *message,
 	if (dbus_message_has_error(message))
 		return;
 
-	l_dbus_message_get_arguments(message, "oas", &path, &interfaces);
+	if (!l_dbus_message_get_arguments(message, "oas", &path, &interfaces))
+		return;
 
 	while (l_dbus_message_iter_next_entry(&interfaces, &interface)) {
 		proxy = proxy_interface_find(interface, path);
-- 
2.13.6

[PATCH 3/8] client: Ignore invalid agent requests

[Tim Kourt]

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

---
 client/agent.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/client/agent.c b/client/agent.c
index 5ed6538f..a64d71c5 100644
--- a/client/agent.c
+++ b/client/agent.c
@@ -112,7 +112,9 @@ static struct l_dbus_message *request_passphrase_method_call(
 					"'--"COMMAND_OPTION_PASSPHRASE"' "
 						"command-line option.\n");
 
-	l_dbus_message_get_arguments(message, "o", &path);
+	if (!l_dbus_message_get_arguments(message, "o", &path))
+		return NULL;
+
 	if (!path)
 		return NULL;
 
@@ -153,7 +155,9 @@ static struct l_dbus_message *request_private_key_passphrase_method_call(
 					"'--"COMMAND_OPTION_PASSPHRASE"' "
 						"command-line option.\n");
 
-	l_dbus_message_get_arguments(message, "o", &path);
+	if (!l_dbus_message_get_arguments(message, "o", &path))
+		return NULL;
+
 	if (!path)
 		return NULL;
 
@@ -216,7 +220,9 @@ static struct l_dbus_message *request_username_and_password_method_call(
 					"'--"COMMAND_OPTION_PASSWORD"' "
 						"command-line option.\n");
 
-	l_dbus_message_get_arguments(message, "o", &path);
+	if (!l_dbus_message_get_arguments(message, "o", &path))
+		return NULL;
+
 	if (!path)
 		return NULL;
 
@@ -275,7 +281,9 @@ static struct l_dbus_message *request_user_password_method_call(
 					"'--"COMMAND_OPTION_PASSWORD"' "
 					"command-line option.\n");
 
-	l_dbus_message_get_arguments(message, "os", &path, &username);
+	if (!l_dbus_message_get_arguments(message, "os", &path, &username))
+		return NULL;
+
 	if (!path || !username)
 		return NULL;
 
-- 
2.13.6

[PATCH 4/8] hotspot: Fix mem leak on failed hotspot config

[Tim Kourt]

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

---
 src/hotspot.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/hotspot.c b/src/hotspot.c
index 60a2e2de..8d2bb04a 100644
--- a/src/hotspot.c
+++ b/src/hotspot.c
@@ -423,8 +423,10 @@ static void hs20_dir_watch_cb(const char *filename,
 		}
 
 		config = hs20_config_new(new, full_path);
-		if (!config)
-			break;
+		if (!config) {
+			l_settings_free(new);
+			return;
+		}
 
 		l_queue_push_head(hs20_settings, config);
 
-- 
2.13.6

[PATCH 5/8] hotspot Eliminate double assignment of variable

[Tim Kourt]

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

---
 src/hotspot.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/hotspot.c b/src/hotspot.c
index 8d2bb04a..a101cbb0 100644
--- a/src/hotspot.c
+++ b/src/hotspot.c
@@ -320,7 +320,7 @@ static struct hs20_config *hs20_config_new(struct l_settings *settings,
 	char **nai_realms = NULL;
 	const char *rc_str;
 	char *name;
-	bool autoconnect = true;
+	bool autoconnect;
 
 	/* One of HESSID, NAI realms, or Roaming Consortium must be included */
 	hessid_str = l_settings_get_string(settings, "Hotspot", "HESSID");
@@ -330,7 +330,9 @@ static struct hs20_config *hs20_config_new(struct l_settings *settings,
 
 	rc_str = l_settings_get_value(settings, "Hotspot", "RoamingConsortium");
 
-	l_settings_get_bool(settings, "Settings", "Autoconnect", &autoconnect);
+	if (!l_settings_get_bool(settings, "Settings", "Autoconnect",
+								&autoconnect))
+		autoconnect = true;
 
 	name = l_settings_get_string(settings, "Hotspot", "Name");
 
-- 
2.13.6

[PATCH 6/8] hwsim: Fix potential memory leak

[Tim Kourt]

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

If msg has multiple RADIO NAME attributes, memory leak occurs.
---
 tools/hwsim.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/hwsim.c b/tools/hwsim.c
index e9ba1f95..500762db 100644
--- a/tools/hwsim.c
+++ b/tools/hwsim.c
@@ -312,11 +312,12 @@ static void list_callback(struct l_genl_msg *msg, void *user_data)
 			break;
 
 		case HWSIM_ATTR_RADIO_NAME:
-			hwname = l_malloc(len + 1);
-			if (hwname) {
+			if (!hwname) {
+				hwname = l_new(char, len + 1);
+
 				strncpy(hwname, data, len);
-				hwname[len] = '\0';
 			}
+
 			break;
 
 		default:
-- 
2.13.6

[PATCH 7/8] client: Check family name before comparison

[Tim Kourt]

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

---
 client/display.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/client/display.c b/client/display.c
index 0faa0840..b22b6e90 100644
--- a/client/display.c
+++ b/client/display.c
@@ -180,7 +180,8 @@ void display_refresh_set_cmd(const char *family, const char *entity,
 		return;
 	}
 
-	if (display_refresh.family && !strcmp(display_refresh.family, family)) {
+	if (display_refresh.family && family &&
+				!strcmp(display_refresh.family, family)) {
 		struct l_string *buf = l_string_new(128);
 		L_AUTO_FREE_VAR(char *, args);
 		char *prompt;
@@ -205,10 +206,10 @@ void display_refresh_set_cmd(const char *family, const char *entity,
 
 		args = l_string_unwrap(buf);
 
-		prompt = l_strdup_printf(IWD_PROMPT"%s%s%s %s %s\n",
-					family ? : "",
-					entity ? " " : "", entity ? : "",
-					cmd->cmd ? : "", args ? : "");
+		prompt = l_strdup_printf(IWD_PROMPT"%s%s%s %s %s\n", family,
+						entity ? " " : "",
+						entity ? : "",
+						cmd->cmd ? : "", args ? : "");
 
 		l_queue_push_tail(display_refresh.redo_entries, prompt);
 		display_refresh.undo_lines++;
-- 
2.13.6

[PATCH 8/8] ofono: Validate message parsing

[Tim Kourt]

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

---
 plugins/ofono.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/plugins/ofono.c b/plugins/ofono.c
index 3120ea70..b13b3fb9 100644
--- a/plugins/ofono.c
+++ b/plugins/ofono.c
@@ -624,7 +624,8 @@ static void interfaces_changed_cb(struct l_dbus_message *message,
 	struct l_dbus_message_iter value;
 	const char *key;
 
-	l_dbus_message_get_arguments(message, "sv", &key, &value);
+	if (!l_dbus_message_get_arguments(message, "sv", &key, &value))
+		return;
 
 	if (!strcmp(key, "Interfaces"))
 		parse_interfaces(&value, modem);
@@ -718,10 +719,11 @@ static void get_modems_cb(struct l_dbus_message *reply, void *user_data)
 
 	modems = l_queue_new();
 
-	l_dbus_message_get_arguments(reply, "a(oa{sv})", &modem_list);
-
-	while (l_dbus_message_iter_next_entry(&modem_list, &path, &props))
-		parse_modem(path, &props);
+	if (l_dbus_message_get_arguments(reply, "a(oa{sv})", &modem_list)) {
+		while (l_dbus_message_iter_next_entry(&modem_list, &path,
+									&props))
+			parse_modem(path, &props);
+	}
 
 	/* watch for modems being added/removed */
 	modem_add_watch = l_dbus_add_signal_watch(dbus_get_bus(),
-- 
2.13.6

Re: [PATCH 1/8] client: Treat invalid DBus reply same as an error

[Denis Kenzior]

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

Hi Tim,

On 10/23/19 3:23 PM, Tim Kourt wrote:
> ---
>   client/dbus-proxy.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 

Patch 1 & 2 applied, thanks.

Regards,
-Denis

Re: [PATCH 3/8] client: Ignore invalid agent requests

[Denis Kenzior]

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

Hi Tim,

On 10/23/19 3:23 PM, Tim Kourt wrote:
> ---
>   client/agent.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/client/agent.c b/client/agent.c
> index 5ed6538f..a64d71c5 100644
> --- a/client/agent.c
> +++ b/client/agent.c
> @@ -112,7 +112,9 @@ static struct l_dbus_message *request_passphrase_method_call(
>   					"'--"COMMAND_OPTION_PASSPHRASE"' "
>   						"command-line option.\n");
>   
> -	l_dbus_message_get_arguments(message, "o", &path);
> +	if (!l_dbus_message_get_arguments(message, "o", &path))
> +		return NULL;
> +

This isn't actually correct.  If the method call fails to parse, then 
returning an error here just means that no reply is sent (and it will 
time out in 30 seconds or so, depending on dbus policy).  You probably 
want to send an error instead.

Returning NULL is generally only used for asynchronous callbacks.

>   	if (!path)
>   		return NULL;
>   
> @@ -153,7 +155,9 @@ static struct l_dbus_message *request_private_key_passphrase_method_call(
>   					"'--"COMMAND_OPTION_PASSPHRASE"' "
>   						"command-line option.\n");
>   
> -	l_dbus_message_get_arguments(message, "o", &path);
> +	if (!l_dbus_message_get_arguments(message, "o", &path))
> +		return NULL;
> +
>   	if (!path)
>   		return NULL;
>   
> @@ -216,7 +220,9 @@ static struct l_dbus_message *request_username_and_password_method_call(
>   					"'--"COMMAND_OPTION_PASSWORD"' "
>   						"command-line option.\n");
>   
> -	l_dbus_message_get_arguments(message, "o", &path);
> +	if (!l_dbus_message_get_arguments(message, "o", &path))
> +		return NULL;
> +
>   	if (!path)
>   		return NULL;
>   
> @@ -275,7 +281,9 @@ static struct l_dbus_message *request_user_password_method_call(
>   					"'--"COMMAND_OPTION_PASSWORD"' "
>   					"command-line option.\n");
>   
> -	l_dbus_message_get_arguments(message, "os", &path, &username);
> +	if (!l_dbus_message_get_arguments(message, "os", &path, &username))
> +		return NULL;
> +
>   	if (!path || !username)
>   		return NULL;
>   
> 

Same for all of these.

Regards,
-Denis

Re: [PATCH 4/8] hotspot: Fix mem leak on failed hotspot config

[Denis Kenzior]

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

Hi Tim,

On 10/23/19 3:23 PM, Tim Kourt wrote:
> ---
>   src/hotspot.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 

I rewrote this patch slightly and applied, thanks.

Regards,
-Denis

Re: [PATCH 5/8] hotspot Eliminate double assignment of variable

[Denis Kenzior]

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

Hi Tim,

On 10/23/19 3:24 PM, Tim Kourt wrote:
> ---
>   src/hotspot.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 

Patches 5-8 applied, thanks.

Regards,
-Denis