[PATCH BlueZ] client: Fix uninitialized read in attribute handle

[PATCH BlueZ] client: Fix uninitialized read in attribute handle

[Alicia Boya Garcia]

When services, characteristics and descriptors were parsed from DBus
proxies the client code was calling the print code without initializing
the `handle` field, which the print functions use.

This resulted in semi-random or zero handles in all attributes when
using gatt.list-attributes in bluetoothctl, depending on compilation
flags.

This patch fixes the problem by parsing the handle from the DBus proxy
path.
---
 client/gatt.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/client/gatt.c b/client/gatt.c
index 21fd38ecf..4c074706c 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -158,6 +158,15 @@ static void print_inc_service(struct service *service, const char *description)
 					service->uuid, text);
 }
 
+static uint16_t handle_from_path(const char *path)
+{
+	const char *number = path + strlen(path) - 4;
+	if (number < path)
+		return 0;
+
+	return (uint16_t) strtol(number, NULL, 16);
+}
+
 static void print_service_proxy(GDBusProxy *proxy, const char *description)
 {
 	struct service service;
@@ -178,6 +187,7 @@ static void print_service_proxy(GDBusProxy *proxy, const char *description)
 	service.path = (char *) g_dbus_proxy_get_path(proxy);
 	service.uuid = (char *) uuid;
 	service.primary = primary;
+	service.handle = handle_from_path(service.path);
 
 	print_service(&service, description);
 }
@@ -259,6 +269,7 @@ static void print_characteristic(GDBusProxy *proxy, const char *description)
 
 	chrc.path = (char *) g_dbus_proxy_get_path(proxy);
 	chrc.uuid = (char *) uuid;
+	chrc.handle = handle_from_path(chrc.path);
 
 	print_chrc(&chrc, description);
 }
@@ -352,6 +363,7 @@ static void print_descriptor(GDBusProxy *proxy, const char *description)
 
 	desc.path = (char *) g_dbus_proxy_get_path(proxy);
 	desc.uuid = (char *) uuid;
+	desc.handle = handle_from_path(desc.path);
 
 	print_desc(&desc, description);
 }
-- 
2.37.1

[PATCH BlueZ] client: Fix uninitialized read in attribute handle

[Alicia Boya Garcia]

When services, characteristics and descriptors were parsed from DBus
proxies the client code was calling the print code without initializing
the `handle` field, which the print functions use.

This resulted in semi-random or zero handles in all attributes when
using gatt.list-attributes in bluetoothctl, depending on compilation
flags.

This patch fixes the problem by parsing the handle from the DBus proxy
path.
---
 client/gatt.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/client/gatt.c b/client/gatt.c
index 4c1efaf75..aaad786b2 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -158,6 +158,16 @@ static void print_inc_service(struct service *service, const char *description)
 					service->uuid, text);
 }
 
+static uint16_t handle_from_path(const char *path)
+{
+	const char *number = path + strlen(path) - 4;
+
+	if (number < path)
+		return 0;
+
+	return (uint16_t) strtol(number, NULL, 16);
+}
+
 static void print_service_proxy(GDBusProxy *proxy, const char *description)
 {
 	struct service service;
@@ -179,6 +189,7 @@ static void print_service_proxy(GDBusProxy *proxy, const char *description)
 	service.path = (char *) g_dbus_proxy_get_path(proxy);
 	service.uuid = (char *) uuid;
 	service.primary = primary;
+	service.handle = handle_from_path(service.path);
 
 	print_service(&service, description);
 }
@@ -261,6 +272,7 @@ static void print_characteristic(GDBusProxy *proxy, const char *description)
 	memset(&chrc, 0, sizeof(chrc));
 	chrc.path = (char *) g_dbus_proxy_get_path(proxy);
 	chrc.uuid = (char *) uuid;
+	chrc.handle = handle_from_path(chrc.path);
 
 	print_chrc(&chrc, description);
 }
@@ -355,6 +367,7 @@ static void print_descriptor(GDBusProxy *proxy, const char *description)
 	memset(&desc, 0, sizeof(desc));
 	desc.path = (char *) g_dbus_proxy_get_path(proxy);
 	desc.uuid = (char *) uuid;
+	desc.handle = handle_from_path(desc.path);
 
 	print_desc(&desc, description);
 }
-- 
2.37.1

RE: [BlueZ] client: Fix uninitialized read in attribute handle

[bluez.test.bot]

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

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=666005

---Test result---

Test Summary:
CheckPatch                    PASS      2.06 seconds
GitLint                       PASS      1.07 seconds
Prep - Setup ELL              PASS      29.52 seconds
Build - Prep                  PASS      0.83 seconds
Build - Configure             PASS      8.97 seconds
Build - Make                  PASS      879.44 seconds
Make Check                    PASS      12.32 seconds
Make Check w/Valgrind         PASS      288.51 seconds
Make Distcheck                PASS      241.98 seconds
Build w/ext ELL - Configure   PASS      8.85 seconds
Build w/ext ELL - Make        PASS      82.41 seconds
Incremental Build w/ patches  PASS      0.00 seconds
Scan Build                    PASS      497.89 seconds



---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ] client: Fix uninitialized read in attribute handle

[Luiz Augusto von Dentz]

Hi Alicia,

On Mon, Aug 8, 2022 at 2:30 AM Alicia Boya Garcia <ntrrgc@gmail.com> wrote:
>
> When services, characteristics and descriptors were parsed from DBus
> proxies the client code was calling the print code without initializing
> the `handle` field, which the print functions use.
>
> This resulted in semi-random or zero handles in all attributes when
> using gatt.list-attributes in bluetoothctl, depending on compilation
> flags.
>
> This patch fixes the problem by parsing the handle from the DBus proxy
> path.
> ---
>  client/gatt.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/client/gatt.c b/client/gatt.c
> index 4c1efaf75..aaad786b2 100644
> --- a/client/gatt.c
> +++ b/client/gatt.c
> @@ -158,6 +158,16 @@ static void print_inc_service(struct service *service, const char *description)
>                                         service->uuid, text);
>  }
>
> +static uint16_t handle_from_path(const char *path)
> +{
> +       const char *number = path + strlen(path) - 4;
> +
> +       if (number < path)
> +               return 0;
> +
> +       return (uint16_t) strtol(number, NULL, 16);
> +}

Object paths are subject to change so we shouldn't consider them as
APIs, so what we should aim to do if we really want to expose handles
of remote attributes is to have it as a D-Bus property Handle like we
do for the servers:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/gatt-api.txt#n53

>  static void print_service_proxy(GDBusProxy *proxy, const char *description)
>  {
>         struct service service;
> @@ -179,6 +189,7 @@ static void print_service_proxy(GDBusProxy *proxy, const char *description)
>         service.path = (char *) g_dbus_proxy_get_path(proxy);
>         service.uuid = (char *) uuid;
>         service.primary = primary;
> +       service.handle = handle_from_path(service.path);
>
>         print_service(&service, description);
>  }
> @@ -261,6 +272,7 @@ static void print_characteristic(GDBusProxy *proxy, const char *description)
>         memset(&chrc, 0, sizeof(chrc));
>         chrc.path = (char *) g_dbus_proxy_get_path(proxy);
>         chrc.uuid = (char *) uuid;
> +       chrc.handle = handle_from_path(chrc.path);
>
>         print_chrc(&chrc, description);
>  }
> @@ -355,6 +367,7 @@ static void print_descriptor(GDBusProxy *proxy, const char *description)
>         memset(&desc, 0, sizeof(desc));
>         desc.path = (char *) g_dbus_proxy_get_path(proxy);
>         desc.uuid = (char *) uuid;
> +       desc.handle = handle_from_path(desc.path);
>
>         print_desc(&desc, description);
>  }
> --
> 2.37.1
>


-- 
Luiz Augusto von Dentz