diff mbox

new_dbus_handlers: Clear errno

Message ID 20121109015139.D066B200501@clearcreek.mtv.corp.google.com
State Accepted
Commit 45ac5793fcbe3785282ebd190132dd59fb06c707
Headers show

Commit Message

Paul Stewart Nov. 8, 2012, 7:43 p.m. UTC 
There are a few instances where dbus handlers test the value
of errno to test whether strtoul completes successfully.
Since strtoul does not clear errno, and there's no strong
reason to suspect that errno is already clear, it is safer
to clear it right before calling strtoul.  Also, any failure
in strtoul (setting errno non-zero) should be considered a
failure.

While testing using dbus-send, I found that a malformed
network path can cause a crash due to net_id being left
NULL.  We should test for this before calling strtoul
on it.

Tested with:

dbus-send --system --dest=fi.w1.wpa_supplicant1 --print-reply \
    /fi/w1/wpa_supplicant1/Interfaces/0 \
    org.freedesktop.DBus.Properties.Get \
    string:fi.w1.wpa_supplicant1.Interface string:Networks
dbus-send --system --dest=fi.w1.wpa_supplicant1 --print-reply \
   /fi/w1/wpa_supplicant1/Interfaces/0 \
   fi.w1.wpa_supplicant1.Interface.RemoveNetwork \
   objpath:/fi/w1/wpa_supplicant1/Interfaces/0/Networks/0
dbus-send --system --dest=fi.w1.wpa_supplicant1 --print-reply \
   /fi/w1/wpa_supplicant1/Interfaces/0 \
   fi.w1.wpa_supplicant1.Interface.RemoveNetwork \
   objpath:/fi/w1/wpa_supplicant1/Interfaces/0/Networks/0
dbus-send --system --dest=fi.w1.wpa_supplicant1 --print-reply \
   /fi/w1/wpa_supplicant1/Interfaces/0 \
   fi.w1.wpa_supplicant1.Interface.RemoveNetwork \
   objpath:/fi/w1/wpa_supplicant1/Interfaces/0

Signed-hostap: Paul Stewart <pstew@chromium.org>
---
 wpa_supplicant/dbus/dbus_new_handlers.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

Comments

Jouni Malinen Nov. 11, 2012, 9:20 a.m. UTC | #1 
On Thu, Nov 08, 2012 at 11:43:19AM -0800, Paul Stewart wrote:
> There are a few instances where dbus handlers test the value
> of errno to test whether strtoul completes successfully.
> Since strtoul does not clear errno, and there's no strong
> reason to suspect that errno is already clear, it is safer
> to clear it right before calling strtoul.  Also, any failure
> in strtoul (setting errno non-zero) should be considered a
> failure.
> 
> While testing using dbus-send, I found that a malformed
> network path can cause a crash due to net_id being left
> NULL.  We should test for this before calling strtoul
> on it.

Thanks, applied.
diff mbox

Patch

diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c b/wpa_supplicant/dbus/dbus_new_handlers.c
index e100df2..65f7226 100644
--- a/wpa_supplicant/dbus/dbus_new_handlers.c
+++ b/wpa_supplicant/dbus/dbus_new_handlers.c
@@ -1492,13 +1492,15 @@  DBusMessage * wpas_dbus_handler_remove_network(DBusMessage *message,
 	/* Extract the network ID and ensure the network */
 	/* is actually a child of this interface */
 	iface = wpas_dbus_new_decompose_object_path(op, 0, &net_id, NULL);
-	if (iface == NULL || os_strcmp(iface, wpa_s->dbus_new_path) != 0) {
+	if (iface == NULL || net_id == NULL ||
+	    os_strcmp(iface, wpa_s->dbus_new_path) != 0) {
 		reply = wpas_dbus_error_invalid_args(message, op);
 		goto out;
 	}
 
+	errno = 0;
 	id = strtoul(net_id, NULL, 10);
-	if (errno == EINVAL) {
+	if (errno != 0) {
 		reply = wpas_dbus_error_invalid_args(message, op);
 		goto out;
 	}
@@ -1592,13 +1594,15 @@  DBusMessage * wpas_dbus_handler_select_network(DBusMessage *message,
 	/* Extract the network ID and ensure the network */
 	/* is actually a child of this interface */
 	iface = wpas_dbus_new_decompose_object_path(op, 0, &net_id, NULL);
-	if (iface == NULL || os_strcmp(iface, wpa_s->dbus_new_path) != 0) {
+	if (iface == NULL || net_id == NULL ||
+	    os_strcmp(iface, wpa_s->dbus_new_path) != 0) {
 		reply = wpas_dbus_error_invalid_args(message, op);
 		goto out;
 	}
 
+	errno = 0;
 	id = strtoul(net_id, NULL, 10);
-	if (errno == EINVAL) {
+	if (errno != 0) {
 		reply = wpas_dbus_error_invalid_args(message, op);
 		goto out;
 	}
@@ -1647,13 +1651,15 @@  DBusMessage * wpas_dbus_handler_network_reply(DBusMessage *message,
 	/* Extract the network ID and ensure the network */
 	/* is actually a child of this interface */
 	iface = wpas_dbus_new_decompose_object_path(op, 0, &net_id, NULL);
-	if (iface == NULL || os_strcmp(iface, wpa_s->dbus_new_path) != 0) {
+	if (iface == NULL || net_id == NULL ||
+	    os_strcmp(iface, wpa_s->dbus_new_path) != 0) {
 		reply = wpas_dbus_error_invalid_args(message, op);
 		goto out;
 	}
 
+	errno = 0;
 	id = strtoul(net_id, NULL, 10);
-	if (errno == EINVAL) {
+	if (errno != 0) {
 		reply = wpas_dbus_error_invalid_args(message, net_id);
 		goto out;
 	}