Hi,
Good catch on type checking.
On Thu, 2015-09-03 at 16:29 +0200, Marcus Folkesson wrote:
> dbus library calls abort() if it got a non-expected type passed to it.
> ---
> src/agent-connman.c | 89
> +++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 86 insertions(+), 3 deletions(-)
>
> diff --git a/src/agent-connman.c b/src/agent-connman.c
> index 2d714b5..381fd25 100644
> --- a/src/agent-connman.c
> +++ b/src/agent-connman.c
> @@ -54,6 +54,34 @@ static bool check_reply_has_dict(DBusMessage *reply)
> return false;
> }
>
> +static bool check_message_is_basic_type(DBusMessageIter *entry)
> +{
> + int type = dbus_message_iter_get_arg_type(entry);
> + switch (type) {
> + case DBUS_TYPE_BYTE:
> + case DBUS_TYPE_BOOLEAN:
> + case DBUS_TYPE_INT16:
> + case DBUS_TYPE_UINT16:
> + case DBUS_TYPE_INT32:
> + case DBUS_TYPE_UINT32:
> + case DBUS_TYPE_INT64:
> + case DBUS_TYPE_UINT64:
> + case DBUS_TYPE_DOUBLE:
> + case DBUS_TYPE_STRING:
> + case DBUS_TYPE_OBJECT_PATH:
> + case DBUS_TYPE_SIGNATURE:
> + case DBUS_TYPE_UNIX_FD:
> + return true;
> +
> + case DBUS_TYPE_VARIANT:
> + case DBUS_TYPE_ARRAY:
> + case DBUS_TYPE_STRUCT:
> + case DBUS_TYPE_DICT_ENTRY:
> + default:
> + return false;
> + }
> +}
> +
> struct request_input_reply {
> struct connman_service *service;
> struct connman_peer *peer;
> @@ -110,7 +138,14 @@ static void request_input_passphrase_reply(DBusMessage
> *reply, void *user_data)
> if (dbus_message_iter_get_arg_type(&entry)
> != DBUS_TYPE_VARIANT)
> break;
> +
> dbus_message_iter_recurse(&entry, &value);
> + if (!check_message_is_basic_type(&value)) {
> + error = CONNMAN_ERROR_INTERFACE
> ".InvalidArguments";
> + values_received = false;
> + break;
> + }
> +
In this code and the following below, it is true that the basic types
need checking. However, if the basic type is sent as int, we'd still do
the wrong things here assuming it's correct. So instead of checking for
all basic types, it'd be better to break out if the type is not exactly
the one expected, i.e. string in most cases.
> dbus_message_iter_get_basic(&value, &identity);
>
> } else if (g_str_equal(key, "Passphrase")) {
> @@ -118,7 +153,14 @@ static void request_input_passphrase_reply(DBusMessage
> *reply, void *user_data)
> if (dbus_message_iter_get_arg_type(&entry)
> != DBUS_TYPE_VARIANT)
> break;
> +
> dbus_message_iter_recurse(&entry, &value);
> + if (!check_message_is_basic_type(&value)) {
> + error = CONNMAN_ERROR_INTERFACE
> ".InvalidArguments";
> + values_received = false;
> + break;
> + }
> +
> dbus_message_iter_get_basic(&value, &passphrase);
>
> } else if (g_str_equal(key, "WPS")) {
> @@ -128,7 +170,14 @@ static void request_input_passphrase_reply(DBusMessage
> *reply, void *user_data)
> if (dbus_message_iter_get_arg_type(&entry)
> != DBUS_TYPE_VARIANT)
> break;
> +
> dbus_message_iter_recurse(&entry, &value);
> + if (!check_message_is_basic_type(&value)) {
> + error = CONNMAN_ERROR_INTERFACE
> ".InvalidArguments";
> + values_received = false;
> + break;
> + }
> +
> dbus_message_iter_get_basic(&value, &wpspin);
> break;
> } else if (g_str_equal(key, "Name")) {
> @@ -136,7 +185,14 @@ static void request_input_passphrase_reply(DBusMessage
> *reply, void *user_data)
> if (dbus_message_iter_get_arg_type(&entry)
> != DBUS_TYPE_VARIANT)
> break;
> +
> dbus_message_iter_recurse(&entry, &value);
> + if (!check_message_is_basic_type(&value)) {
> + error = CONNMAN_ERROR_INTERFACE
> ".InvalidArguments";
> + values_received = false;
> + break;
> + }
> +
> dbus_message_iter_get_basic(&value, &name);
> name_len = strlen(name);
> } else if (g_str_equal(key, "SSID")) {
> @@ -144,16 +200,25 @@ static void request_input_passphrase_reply(DBusMessage
> *reply, void *user_data)
>
> dbus_message_iter_next(&entry);
> if (dbus_message_iter_get_arg_type(&entry)
> - != DBUS_TYPE_VARIANT)
> + != DBUS_TYPE_VARIANT) {
> + error = CONNMAN_ERROR_INTERFACE
> ".InvalidArguments";
> + values_received = false;
> break;
> + }
> dbus_message_iter_recurse(&entry, &value);
> if (dbus_message_iter_get_arg_type(&value)
> - != DBUS_TYPE_ARRAY)
> + != DBUS_TYPE_ARRAY) {
> + error = CONNMAN_ERROR_INTERFACE
> ".InvalidArguments";
> + values_received = false;
> break;
> + }
> dbus_message_iter_recurse(&value, &array_iter);
> if (dbus_message_iter_get_arg_type(&array_iter)
> - != DBUS_TYPE_BYTE)
> + != DBUS_TYPE_BYTE) {
> + error = CONNMAN_ERROR_INTERFACE
> ".InvalidArguments";
> + values_received = false;
> break;
> + }
> dbus_message_iter_get_fixed_array(&array_iter, &name,
> &name_len);
> }
> @@ -401,7 +466,14 @@ static void request_input_login_reply(DBusMessage
> *reply, void *user_data)
> if (dbus_message_iter_get_arg_type(&entry)
> != DBUS_TYPE_VARIANT)
> break;
> +
> dbus_message_iter_recurse(&entry, &value);
> + if (!check_message_is_basic_type(&value)) {
> + error = CONNMAN_ERROR_INTERFACE
> ".InvalidArguments";
> + values_received = false;
> + break;
> + }
> +
> dbus_message_iter_get_basic(&value, &username);
>
> } else if (g_str_equal(key, "Password")) {
> @@ -410,6 +482,12 @@ static void request_input_login_reply(DBusMessage
> *reply, void *user_data)
> DBUS_TYPE_VARIANT)
> break;
> dbus_message_iter_recurse(&entry, &value);
> + if (!check_message_is_basic_type(&value)) {
> + error = CONNMAN_ERROR_INTERFACE
> ".InvalidArguments";
> + values_received = false;
> + break;
> + }
> +
> dbus_message_iter_get_basic(&value, &password);
> }
>
> @@ -718,6 +796,11 @@ static void request_peer_authorization_reply(DBusMessage
> *reply,
> != DBUS_TYPE_VARIANT)
> break;
> dbus_message_iter_recurse(&entry, &value);
> + if (!check_message_is_basic_type(&value)) {
> + error = CONNMAN_ERROR_INTERFACE
> ".InvalidArguments";
> + break;
> + }
> +
> dbus_message_iter_get_basic(&value, &wpspin);
> break;
> }
Patrik
_______________________________________________
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman
