usb: defensively destructure GVariants to prevent crashes

The data is coming in from the outside, so it is untrusted. We check for
the size of the variant, because otherwise we might crash at runtime,
since g_variant_get_child might leave variables, i.e. GVariantIters,
uninitialised (NULL) which then causes g_variant_iter_loop to crash,
since it doesn't check for NULL.

The check for g_variant_n_children is arguably ugly, because
g_variant_get_child will eventually perform that check.
So we focus on the returned NULL in case of g_variant_get_child_value
or, in case of g_variant_get_child, whether the memory has not changed.

The documentation is, for my taste, not as clear as it could or should be.
It mentions that it is "an error" to provide an index higher than what
the GVariant contains. But it doesn't mention how it communicates that
error nor how to deal with it.

1 files changed, 8 insertions, 0 deletions

diff --git a/plugins/usb-protection/gsd-usb-protection-manager.c b/plugins/usb-protection/gsd-usb-protection-manager.c
index 142760ac..accaa8e7 100644
--- a/plugins/usb-protection/gsd-usb-protection-manager.c
+++ b/plugins/usb-protection/gsd-usb-protection-manager.c
@@ -207,6 +207,7 @@ is_usbguard_allow_rule_present (GVariant *rules)
         g_debug ("Detecting rule...");
 
         g_variant_get (rules, "a(us)", &iter);
+        g_return_val_if_fail (iter != NULL, FALSE);
         while (g_variant_iter_loop (iter, "(us)", &number, &value)) {
                 if (g_strcmp0 (value, ALLOW_ALL) == 0) {
                         g_debug ("Detected rule!");
@@ -237,6 +238,7 @@ usbguard_listrules_cb (GObject      *source_object,
 
         rules = g_variant_get_child_value (result, 0);
         g_variant_unref (result);
+        g_return_if_fail (rules != NULL);
         if (!is_usbguard_allow_rule_present (rules))
                 add_usbguard_allow_rule (user_data);
 
@@ -440,6 +442,7 @@ is_hid_or_hub (GVariant *device,
         }
 
         g_variant_get_child (device, PRESENCE_ATTRIBUTES, "a{ss}", &iter);
+        g_return_val_if_fail (iter != NULL, FALSE);
         while (g_variant_iter_loop (iter, "{ss}", &name, &value)) {
                 if (g_strcmp0 (name, WITH_INTERFACE) == 0) {
                         g_auto(GStrv) interfaces_splitted = NULL;
@@ -466,6 +469,7 @@ is_hardwired (GVariant *device)
         g_autofree gchar *value = NULL;
 
         g_variant_get_child (device, PRESENCE_ATTRIBUTES, "a{ss}", &iter);
+        g_return_val_if_fail (iter != NULL, FALSE);
         while (g_variant_iter_loop (iter, "{ss}", &name, &value)) {
                 if (g_strcmp0 (name, WITH_CONNECT_TYPE) == 0) {
                         return g_strcmp0 (value, "hardwired") == 0;
@@ -483,6 +487,7 @@ auth_device (GsdUsbProtectionManager *manager,
         if (manager->usb_protection_devices == NULL)
                 return;
 
+        g_return_if_fail (g_variant_n_children (device) >= POLICY_DEVICE_ID);
         g_variant_get_child (device, POLICY_DEVICE_ID, "u", &device_id);
         g_debug ("Authorizing device %u", device_id);
         authorize_device(manager->usb_protection_devices,
@@ -538,6 +543,7 @@ on_usbguard_signal (GDBusProxy *proxy,
                 return;
         }
 
+        g_return_if_fail (g_variant_n_children (parameters) >= PRESENCE_EVENT);
         g_variant_get_child (parameters, PRESENCE_EVENT, "u", &device_event);
         if (device_event != EVENT_INSERT) {
             g_debug ("Device hat not been inserted (%d); ignoring", device_event);
@@ -566,6 +572,7 @@ on_usbguard_signal (GDBusProxy *proxy,
         }
 
         g_variant_get_child (parameters, PRESENCE_ATTRIBUTES, "a{ss}", &iter);
+        g_return_if_fail (iter != NULL);
         while (g_variant_iter_loop (iter, "{ss}", &name, &device_name)) {
                 if (g_strcmp0 (name, NAME) == 0)
                         g_debug ("A new USB device has been connected: %s", device_name);
@@ -658,6 +665,7 @@ on_usb_protection_signal (GDBusProxy *proxy,
                 return;
 
         parameter = g_variant_get_child_value (parameters, 2);
+        g_return_if_fail (parameter != NULL);
         update_usb_protection_store (user_data, parameter);
 
 }