audio/avrcp: Fix crash with invalid UTF-8 item name
v2 cover audio/avrcp: Fix crash with invalid UTF-8 item name
As stated in AVRCP 1.6.2 chapter 6.10.2.3 Media element item, for the Displayable Name Length property, the target device may truncate the item name:
Length of Displayable Name in octets. The name shall be limited such that a response to a GetFolderItems containing one media player item fits within the maximum size of PDU which can be received by the CT.
This truncatation may occur in the middle of a multi-byte character, at least with Samsung Music app, which triggers a DBus assertion and crashes bluetoothd:
profiles/audio/player.c:media_folder_create_item() Din Dhal Jaye
Haye with lyrics | "दिन ढल जाए
हाय" गाने के बो� type audio uid 1
profiles/audio/player.c:media_folder_create_item()
/org/bluez/hci0/dev_24_24_B7_11_82_6C/player0/NowPlaying/item1
profiles/audio/player.c:media_player_set_metadata() Title: Din Dhal
Jaye Haye with lyrics | "दिन ढल जाए हाय"
गाने के बोल | Guide | Dev Anand, Waheeda Rehman
…
arguments to dbus_message_iter_append_basic() were incorrect,
assertion "_dbus_check_is_valid_utf8 (*string_p)" failed in file dbus-message.c line 2775.
This is normally a bug in some application using the D-Bus library.
v1->v2:
- Introduce new strtoutf8() util function to truncate a string before the first non UTF-8 character.
- Use strtoutf8() for AVRCP media element name
- Use strtoutf8() for MCP player name and track title
- Use strtoutf8() for Audio GAP device name
- Use strtoutf8() for EIR device names
Frédéric Danis (5):
shared/util: Add strtoutf8 function audio/avrcp: Fix crash with invalid UTF-8 item name audio/mcp: Use strtoutf8 for player name and track title audio/gap: Use strtoutf8 for GAP device name
eir: Use strtoutf8 for device names
profiles/audio/avrcp.c | 4 +++- profiles/audio/mcp.c | 11 +---------- profiles/gap/gas.c | 11 +---------- src/eir.c | 11 +---------- src/shared/util.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/shared/util.h | 1 + 6 files changed, 49 insertions(+), 31 deletions(-)
Signature · 1 line
2.43.0
PATCH v2 1/5 shared/util: Add strtoutf8 function
This adds the strtoutf8 function that truncate a string before the first non UTF-8 character.
This truncation is done in place.
--- src/shared/util.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/shared/util.h | 1 + 2 files changed, 43 insertions(+)
src/shared/util.c +42 -0
@@ -1959,3 +1959,45 @@ bool argsisutf8(int argc, char *argv[])return true; } + +char *strtoutf8 (char *str,size_t len) +{ +size_t i =0 ; + +while (i < len) { +unsigned char c = str[i]; +size_t size =0 ; + +/* Check the first byte to determine the number of bytes in the +* UTF-8 character. +*/ +if ((c &0x80 ) ==0x00 ) + size =1 ; +else if ((c &0xE0 ) ==0xC0 ) + size =2 ; +else if ((c &0xF0 ) ==0xE0 ) + size =3 ; +else if ((c &0xF8 ) ==0xF0 ) + size =4 ; +else +/* Invalid UTF-8 sequence */ + goto done; + +/* Check the following bytes to ensure they have the correct +* format. +*/ +for (size_t j =1 ; j < size; ++j) { +if (i + j > len || (str[i + j] &0xC0 ) !=0x80 ) +/* Invalid UTF-8 sequence */ + goto done; + } + +/* Move to the next character */ + i += size; + } + +done: +/* Truncate to the longest valid UTF-8 string */ +memset (str + i,0 , len - i); +return str; +}
src/shared/util.h +1 -0
@@ -92,6 +92,7 @@ int strsuffix(const char *str, const char *suffix);char *strstrip (char *str);bool strisutf8 (const char *str,size_t length);bool argsisutf8 (int argc,char *argv[]); +char *strtoutf8 (char *str,size_t len);void *util_malloc (size_t size);void *util_memdup (const void *src,size_t size);
Signature · 1 line
2.43.0
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=979916
---Test result---
Test Summary:
CheckPatch PENDING 0.26 seconds GitLint PENDING 0.28 seconds BuildEll PASS 20.40 seconds BluezMake PASS 2504.14 seconds MakeCheck PASS 19.71 seconds MakeDistcheck PASS 182.72 seconds CheckValgrind PASS 234.41 seconds CheckSmatch PASS 304.83 seconds bluezmakeextell PASS 128.08 seconds IncrementalBuild PENDING 0.32 seconds ScanBuild PASS 904.29 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series Output:
---
Regards,
Linux Bluetooth
Luiz Augusto von Dentz
maintainerRe: [PATCH BlueZ v2 1/5] shared/util: Add strtoutf8 function
In reply to Frédéric DanisHi Frédéric,
Quoted 43 lines · Frédéric Danis
This adds the strtoutf8 function that truncate a string before the first non UTF-8 character.
This truncation is done in place.--- src/shared/util.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/shared/util.h | 1 + 2 files changed, 43 insertions(+)src/shared/util.c +26
-0@@ -1959,3 +1959,45 @@ bool argsisutf8(int argc, char *argv[])return true; } + +char *strtoutf8 (char *str,size_t len) +{ +size_t i =0 ; + +while (i < len) { +unsigned char c = str[i]; +size_t size =0 ; + +/* Check the first byte to determine the number of bytes in the +* UTF-8 character. +*/ +if ((c &0x80 ) ==0x00 ) + size =1 ; +else if ((c &0xE0 ) ==0xC0 ) + size =2 ; +else if ((c &0xF0 ) ==0xE0 ) + size =3 ; +else if ((c &0xF8 ) ==0xF0 ) + size =4 ; +else +/* Invalid UTF-8 sequence */ + goto done; + +/* Check the following bytes to ensure they have the correct +* format.
src/shared/util.c bool argsisutf8(int argc, char *argv[]) · line 1997
Quoted 10 lines
src/shared/util.c +10
-0+*/ +for (size_t j =1 ; j < size; ++j) { +if (i + j > len || (str[i + j] &0xC0 ) !=0x80 ) +/* Invalid UTF-8 sequence */ + goto done; + } + +/* Move to the next character */ + i += size; + }
Perhaps we should split this part on its own function or use strisutf8 but somehow return the last valid character found.
Quoted 22 lines
src/shared/util.c +6
-0+ +done: +/* Truncate to the longest valid UTF-8 string */ +memset (str + i,0 , len - i); +return str; +}src/shared/util.h +1
-1@@ -92,6 +92,7 @@ int strsuffix(const char *str, const char *suffix);char *strstrip (char *str);bool strisutf8 (const char *str,size_t length);bool argsisutf8 (int argc,char *argv[]); +char *strtoutf8 (char *str,size_t len);void *util_malloc (size_t size);void *util_memdup (const void *src,size_t size); --2.43.0
Signature · 1 line
Luiz Augusto von Dentz
PATCH v2 2/5 audio/avrcp: Fix crash with invalid UTF-8 item name
In reply to Frédéric DanisAs stated in AVRCP 1.6.2 chapter 6.10.2.3 Media element item, for the Displayable Name Length property, the target device may truncate the item name:
Length of Displayable Name in octets. The name shall be limited such that a response to a GetFolderItems containing one media player item fits within the maximum size of PDU which can be received by the CT.
This truncatation may occur in the middle of a multi-byte character, at least with Samsung Music app, which triggers a DBus assertion and crashes bluetoothd:
profiles/audio/player.c:media_folder_create_item() Din Dhal Jaye
Haye with lyrics | "दिन ढल जाए
हाय" गाने के बो� type audio uid 1
profiles/audio/player.c:media_folder_create_item()
/org/bluez/hci0/dev_24_24_B7_11_82_6C/player0/NowPlaying/item1
profiles/audio/player.c:media_player_set_metadata() Title: Din Dhal
Jaye Haye with lyrics | "दिन ढल जाए हाय"
गाने के बोल | Guide | Dev Anand, Waheeda Rehman
…
arguments to dbus_message_iter_append_basic() were incorrect,
assertion "_dbus_check_is_valid_utf8 (*string_p)" failed in
file dbus-message.c line 2775.
This is normally a bug in some application using the D-Bus library.
---
profiles/audio/avrcp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)profiles/audio/avrcp.c +3 -1
@@ -2595,8 +2595,10 @@ static struct media_item *parse_media_element(struct avrcp *session,memset (name,0 ,sizeof (name)); namesize =get_be16 (&operands[11 ]); namelen =MIN (namesize,sizeof (name) -1 ); -if (namelen >0 ) +if (namelen >0 ) {memcpy (name, &operands[13 ], namelen); +strtoutf8 (name, namelen); + } count = operands[13 + namesize];
Signature · 1 line
2.43.0
PATCH v2 3/5 audio/mcp: Use strtoutf8 for player name and track title
In reply to Frédéric DanisTruncate the string to first character before invalid UTF-8 one instead of replacing non ascii characters by spaces.
--- profiles/audio/mcp.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)
profiles/audio/mcp.c +1 -10
@@ -72,21 +72,12 @@ static void mcp_debug(const char *str, void *user_data)static char *name2utf8 (const uint8_t *name,uint16_t len) {char utf8_name[HCI_MAX_NAME_LENGTH +2 ]; -int i; - -if (g_utf8_validate ((const char *) name, len,NULL )) -return g_strndup ((char *) name, len); len =MIN (len,sizeof (utf8_name) -1 );memset (utf8_name,0 ,sizeof (utf8_name));strncpy (utf8_name, (char *) name, len); - -/* Assume ASCII, and replace all non-ASCII with spaces */ -for (i =0 ; utf8_name[i] !='\0' ; i++) { -if (!isascii (utf8_name[i])) - utf8_name[i] =' ' ; - } +strtoutf8 (utf8_name, len);/* Remove leading and trailing whitespace characters */ g_strstrip (utf8_name);
Signature · 1 line
2.43.0
PATCH v2 4/5 audio/gap: Use strtoutf8 for GAP device name
In reply to Frédéric DanisTruncate the string to first character before invalid UTF-8 one instead of replacing non ascii characters by spaces.
--- profiles/gap/gas.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)
profiles/gap/gas.c +1 -10
@@ -69,21 +69,12 @@ static void gas_free(struct gas *gas)static char *name2utf8 (const uint8_t *name,uint16_t len) {char utf8_name[HCI_MAX_NAME_LENGTH +2 ]; -int i; - -if (g_utf8_validate ((const char *) name, len,NULL )) -return g_strndup ((char *) name, len); len =MIN (len,sizeof (utf8_name) -1 );memset (utf8_name,0 ,sizeof (utf8_name));strncpy (utf8_name, (char *) name, len); - -/* Assume ASCII, and replace all non-ASCII with spaces */ -for (i =0 ; utf8_name[i] !='\0' ; i++) { -if (!isascii (utf8_name[i])) - utf8_name[i] =' ' ; - } +strtoutf8 (utf8_name, len);/* Remove leading and trailing whitespace characters */ g_strstrip (utf8_name);
Signature · 1 line
2.43.0
PATCH v2 5/5 eir: Use strtoutf8 for device names
In reply to Frédéric DanisTruncate the string to first character before invalid UTF-8 one instead of replacing non ascii characters by spaces.
--- src/eir.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)
src/eir.c +1 -10
@@ -127,19 +127,10 @@ static void eir_parse_uuid128(struct eir_data *eir, const uint8_t *data,static char *name2utf8 (const uint8_t *name,uint8_t len) {char utf8_name[HCI_MAX_NAME_LENGTH +2 ]; -int i; - -if (g_utf8_validate ((const char *) name, len,NULL )) -return g_strndup ((char *) name, len);memset (utf8_name,0 ,sizeof (utf8_name));strncpy (utf8_name, (char *) name, len); - -/* Assume ASCII, and replace all non-ASCII with spaces */ -for (i =0 ; utf8_name[i] !='\0' ; i++) { -if (!isascii (utf8_name[i])) - utf8_name[i] =' ' ; - } +strtoutf8 (utf8_name, len);/* Remove leading and trailing whitespace characters */ g_strstrip (utf8_name);
Signature · 1 line
2.43.0
Quoted 29 lines · Frédéric Danis
As stated in AVRCP 1.6.2 chapter 6.10.2.3 Media element item, for the Displayable Name Length property, the target device may truncate the item name:
Length of Displayable Name in octets. The name shall be limited such
that a response to a GetFolderItems containing one media player item
fits within the maximum size of PDU which can be received by the CT.This truncatation may occur in the middle of a multi-byte character, at least with Samsung Music app, which triggers a DBus assertion and crashes bluetoothd:
profiles/audio/player.c:media_folder_create_item() Din Dhal Jaye Haye with lyrics | "दिन ढल जाए
हाय" गाने के बो� type audio uid 1 profiles/audio/player.c:media_folder_create_item() /org/bluez/hci0/dev_24_24_B7_11_82_6C/player0/NowPlaying/item1 profiles/audio/player.c:media_player_set_metadata() Title: Din Dhal Jaye Haye with lyrics | "दिन ढल जाए हाय"
गाने के बोल | Guide | Dev Anand, Waheeda Rehman
…
arguments todbus_message_iter_append_basic()were incorrect, assertion "_dbus_check_is_valid_utf8(*string_p)" failed in filedbus-message.cline 2775.
This is normally a bug in some application using the D-Bus library.
Quoted 8 lines
v1->v2:
- Introduce newstrtoutf8()util function to truncate a string before
the first non UTF-8 character.
- Usestrtoutf8()for AVRCP media element name
- Usestrtoutf8()for MCP player name and track title
- Usestrtoutf8()for Audio GAP device name
- Usestrtoutf8()for EIR device names
Would be great to have a test case for that new function, with a bunch of the problems you ran into tested for.
Quoted 16 lines
Frédéric Danis (5):
shared/util: Add strtoutf8 function audio/avrcp: Fix crash with invalid UTF-8 item name audio/mcp: Use strtoutf8 for player name and track title audio/gap: Use strtoutf8 for GAP device name
eir: Use strtoutf8 for device namesprofiles/audio/avrcp.c | 4 +++-
profiles/audio/mcp.c | 11 +----------
profiles/gap/gas.c | 11 +----------
src/eir.c | 11 +----------
src/shared/util.c | 42
++++++++++++++++++++++++++++++++++++++++++
src/shared/util.h | 1 +6 files changed, 49 insertions(+), 31 deletions(-)
Luiz Augusto von Dentz
maintainerHi Bastien, Frederic,
Quoted 40 lines · Bastien Nocera
Quoted 29 lines · Frédéric Danis
As stated in AVRCP 1.6.2 chapter 6.10.2.3 Media element item, for the Displayable Name Length property, the target device may truncate the item name:
Length of Displayable Name in octets. The name shall be limited
such
that a response to a GetFolderItems containing one media player
item
fits within the maximum size of PDU which can be received by the
CT.This truncatation may occur in the middle of a multi-byte character, at least with Samsung Music app, which triggers a DBus assertion and crashes bluetoothd:
profiles/audio/player.c:media_folder_create_item() Din Dhal Jaye
Haye with lyrics | "दिन ढल जाए
हाय" गाने के बो� type audio uid 1
profiles/audio/player.c:media_folder_create_item()/org/bluez/hci0/dev_24_24_B7_11_82_6C/player0/NowPlaying/item1profiles/audio/player.c:media_player_set_metadata() Title: Din Dhal
Jaye Haye with lyrics | "दिन ढल जाए हाय"
गाने के बोल | Guide | Dev Anand, Waheeda Rehman
…
arguments todbus_message_iter_append_basic()were incorrect,
assertion "_dbus_check_is_valid_utf8(*string_p)" failed in filedbus-message.cline 2775.
This is normally a bug in some application using the D-Bus library.Quoted 8 lines
v1->v2:
- Introduce newstrtoutf8()util function to truncate a string
before
the first non UTF-8 character.
- Usestrtoutf8()for AVRCP media element name
- Usestrtoutf8()for MCP player name and track title
- Usestrtoutf8()for Audio GAP device name
- Usestrtoutf8()for EIR device names
Quoted 2 lines
Would be great to have a test case for that new function, with a bunch of the problems you ran into tested for.
Yep, we do have the likes of test-eir where we can add tests with names for the devices, we probably want to try with strings where there are invalid UTF-8 and also using other things like UTF16 and other string formats to check that it doesn't crash at least.
Quoted 17 lines
Quoted 16 lines
Frédéric Danis (5):
shared/util: Add strtoutf8 function audio/avrcp: Fix crash with invalid UTF-8 item name audio/mcp: Use strtoutf8 for player name and track title audio/gap: Use strtoutf8 for GAP device name
eir: Use strtoutf8 for device namesprofiles/audio/avrcp.c | 4 +++- profiles/audio/mcp.c | 11 +---------- profiles/gap/gas.c | 11 +---------- src/eir.c | 11 +---------- src/shared/util.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/shared/util.h | 1 + 6 files changed, 49 insertions(+), 31 deletions(-)
Signature · 1 line
Luiz Augusto von Dentz