bluetoothd does not check remote names for valid utf8 data

PATCH David Vrabel 2 participants 3 messages Aug 25, 2010 Bluetooth drivers

Share & tools

David Vrabel

author

2010-08-25 14:31 UTC #0 lore raw

Hi,

bluetoothd does not check in some (all?) places that the remote name reported by a device is valid utf8 data. e.g., extract_eir_name() in src/dbus-hci.c.

The reception of an extended inquiry response containing a name with invalid utf8 data can cause the dbus interface to disappear. This is therefore a denial-of-service vulnerability (at the very least).

The following patch fixes the above problem but there are probably other places where the check needs to be done.

src/dbus-hci.c +2 -0

@@ -450,6 +450,8 @@
 	switch (*type) {
 	case 0x08:
 	case 0x09:
+		if (!g_utf8_validate(data + 2, data[0] - 1, NULL))
+			return strdup("");
 		return strndup((char *) (data + 2), data[0] - 1);
 	}

David

Signature · 6 lines

David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Johan Hedberg

2010-08-25 15:03 UTC #1 lore raw

Hi David,

Quoted 14 lines · David Vrabel

bluetoothd does not check in some (all?) places that the remote name reported by a device is valid utf8 data. e.g., extract_eir_name() in src/dbus-hci.c.
The reception of an extended inquiry response containing a name with invalid utf8 data can cause the dbus interface to disappear. This is therefore a denial-of-service vulnerability (at the very least).
The following patch fixes the above problem but there are probably other places where the check needs to be done.

src/dbus-hci.c line 456

Quoted 7 lines

src/dbus-hci.c +2 -0

@@ -450,6 +450,8 @@
 	switch (*type) {
 	case 0x08:
 	case 0x09:
+		if (!g_utf8_validate(data + 2, data[0] - 1, NULL))
+			return strdup("");
 		return strndup((char *) (data + 2), data[0] - 1);
 	}

Good catch. At least the legacy name queries are already protected (remote_name_information function in security.c) so I think this is the only place missing the UTF-8 validation. However, your patch doesn't compile cleanly so some fine tuning is still needed (always check compilation with "./bootstrap-configure && make" before sending upstream):

src/dbus-hci.c: In function ‘extract_eir_name’:
src/dbus-hci.c:466: error: pointer targets in passing argument 1 of ‘g_utf8_validate’ differ in signedness /usr/include/glib-2.0/glib/gunicode.h:356: note: expected ‘const gchar *’ but argument is of type ‘uint8_t *’ make[1]: *** [src/dbus-hci.o] Error 1

After fixing that, could you prepare the patch through git format-patch so that I can easily apply it using git am? Thanks.

Johan

Johan Hedberg

2010-08-25 21:38 UTC #2 lore raw

Hi again,

Quoted 10 lines · Johan Hedberg

Good catch. At least the legacy name queries are already protected (remote_name_information function in security.c) so I think this is the only place missing the UTF-8 validation. However, your patch doesn't compile cleanly so some fine tuning is still needed (always check compilation with "./bootstrap-configure && make" before sending upstream):
src/dbus-hci.c: In function ‘extract_eir_name’:
src/dbus-hci.c:466: error: pointer targets in passing argument 1 of ‘g_utf8_validate’ differ in signedness /usr/include/glib-2.0/glib/gunicode.h:356: note: expected ‘const gchar *’ but argument is of type ‘uint8_t *’ make[1]: *** [src/dbus-hci.o] Error 1

Since this was the only thing blocking a new release I went ahead and fixed the issue myself. The (fixed) patch is now upstream.

Johan