* [PATCH BlueZ v3 0/7] audio/avrcp: Fix crash with invalid UTF-8 item name
@ 2025-07-08 15:43 Frédéric Danis
2025-07-08 15:43 ` [PATCH BlueZ v3 1/7] shared/util: Add strtoutf8 function Frédéric Danis
` (7 more replies)
0 siblings, 8 replies; 13+ messages in thread
From: Frédéric Danis @ 2025-07-08 15:43 UTC (permalink / raw)
To: linux-bluetooth
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
v2->v3:
- Use strtoutf8() for AD device names
- Add name encoding tests to unit/test-eir
Frédéric Danis (7):
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
shared/ad: Use strtoutf8 for name
unit/test-eir: Add name encoding tests
profiles/audio/avrcp.c | 4 +++-
profiles/audio/mcp.c | 11 +---------
profiles/gap/gas.c | 11 +---------
src/eir.c | 11 +---------
src/shared/ad.c | 7 +-----
src/shared/util.c | 42 +++++++++++++++++++++++++++++++++++
src/shared/util.h | 1 +
unit/test-eir.c | 50 ++++++++++++++++++++++++++++++++++++++++++
8 files changed, 100 insertions(+), 37 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH BlueZ v3 1/7] shared/util: Add strtoutf8 function 2025-07-08 15:43 [PATCH BlueZ v3 0/7] audio/avrcp: Fix crash with invalid UTF-8 item name Frédéric Danis @ 2025-07-08 15:43 ` Frédéric Danis 2025-07-08 17:06 ` audio/avrcp: Fix crash with invalid UTF-8 item name bluez.test.bot 2025-07-08 15:43 ` [PATCH BlueZ v3 2/7] " Frédéric Danis ` (6 subsequent siblings) 7 siblings, 1 reply; 13+ messages in thread From: Frédéric Danis @ 2025-07-08 15:43 UTC (permalink / raw) To: linux-bluetooth 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(+) diff --git a/src/shared/util.c b/src/shared/util.c index 5d3a14d96..5262458cb 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -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; +} diff --git a/src/shared/util.h b/src/shared/util.h index dd357fb93..6fc02a9dc 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* RE: audio/avrcp: Fix crash with invalid UTF-8 item name 2025-07-08 15:43 ` [PATCH BlueZ v3 1/7] shared/util: Add strtoutf8 function Frédéric Danis @ 2025-07-08 17:06 ` bluez.test.bot 0 siblings, 0 replies; 13+ messages in thread From: bluez.test.bot @ 2025-07-08 17:06 UTC (permalink / raw) To: linux-bluetooth, frederic.danis [-- Attachment #1: Type: text/plain, Size: 1261 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=980101 ---Test result--- Test Summary: CheckPatch PENDING 0.25 seconds GitLint PENDING 0.24 seconds BuildEll PASS 20.42 seconds BluezMake PASS 2694.75 seconds MakeCheck PASS 19.84 seconds MakeDistcheck PASS 186.45 seconds CheckValgrind PASS 236.95 seconds CheckSmatch PASS 306.32 seconds bluezmakeextell PASS 127.72 seconds IncrementalBuild PENDING 0.29 seconds ScanBuild PASS 915.07 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH BlueZ v3 2/7] audio/avrcp: Fix crash with invalid UTF-8 item name 2025-07-08 15:43 [PATCH BlueZ v3 0/7] audio/avrcp: Fix crash with invalid UTF-8 item name Frédéric Danis 2025-07-08 15:43 ` [PATCH BlueZ v3 1/7] shared/util: Add strtoutf8 function Frédéric Danis @ 2025-07-08 15:43 ` Frédéric Danis 2025-07-08 15:43 ` [PATCH BlueZ v3 3/7] audio/mcp: Use strtoutf8 for player name and track title Frédéric Danis ` (5 subsequent siblings) 7 siblings, 0 replies; 13+ messages in thread From: Frédéric Danis @ 2025-07-08 15:43 UTC (permalink / raw) To: linux-bluetooth 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. --- profiles/audio/avrcp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c index 831f1dc8b..30997335a 100644 --- a/profiles/audio/avrcp.c +++ b/profiles/audio/avrcp.c @@ -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]; -- 2.43.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH BlueZ v3 3/7] audio/mcp: Use strtoutf8 for player name and track title 2025-07-08 15:43 [PATCH BlueZ v3 0/7] audio/avrcp: Fix crash with invalid UTF-8 item name Frédéric Danis 2025-07-08 15:43 ` [PATCH BlueZ v3 1/7] shared/util: Add strtoutf8 function Frédéric Danis 2025-07-08 15:43 ` [PATCH BlueZ v3 2/7] " Frédéric Danis @ 2025-07-08 15:43 ` Frédéric Danis 2025-07-08 15:43 ` [PATCH BlueZ v3 4/7] audio/gap: Use strtoutf8 for GAP device name Frédéric Danis ` (4 subsequent siblings) 7 siblings, 0 replies; 13+ messages in thread From: Frédéric Danis @ 2025-07-08 15:43 UTC (permalink / raw) To: linux-bluetooth Truncate 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(-) diff --git a/profiles/audio/mcp.c b/profiles/audio/mcp.c index d3ccb97c5..5fe7ef82a 100644 --- a/profiles/audio/mcp.c +++ b/profiles/audio/mcp.c @@ -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); -- 2.43.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH BlueZ v3 4/7] audio/gap: Use strtoutf8 for GAP device name 2025-07-08 15:43 [PATCH BlueZ v3 0/7] audio/avrcp: Fix crash with invalid UTF-8 item name Frédéric Danis ` (2 preceding siblings ...) 2025-07-08 15:43 ` [PATCH BlueZ v3 3/7] audio/mcp: Use strtoutf8 for player name and track title Frédéric Danis @ 2025-07-08 15:43 ` Frédéric Danis 2025-07-08 15:43 ` [PATCH BlueZ v3 5/7] eir: Use strtoutf8 for device names Frédéric Danis ` (3 subsequent siblings) 7 siblings, 0 replies; 13+ messages in thread From: Frédéric Danis @ 2025-07-08 15:43 UTC (permalink / raw) To: linux-bluetooth Truncate 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(-) diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c index 08aaf19cb..74dabf41f 100644 --- a/profiles/gap/gas.c +++ b/profiles/gap/gas.c @@ -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); -- 2.43.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH BlueZ v3 5/7] eir: Use strtoutf8 for device names 2025-07-08 15:43 [PATCH BlueZ v3 0/7] audio/avrcp: Fix crash with invalid UTF-8 item name Frédéric Danis ` (3 preceding siblings ...) 2025-07-08 15:43 ` [PATCH BlueZ v3 4/7] audio/gap: Use strtoutf8 for GAP device name Frédéric Danis @ 2025-07-08 15:43 ` Frédéric Danis 2025-07-08 15:43 ` [PATCH BlueZ v3 6/7] shared/ad: Use strtoutf8 for name Frédéric Danis ` (2 subsequent siblings) 7 siblings, 0 replies; 13+ messages in thread From: Frédéric Danis @ 2025-07-08 15:43 UTC (permalink / raw) To: linux-bluetooth Truncate 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(-) diff --git a/src/eir.c b/src/eir.c index 28b09653c..1e1f76952 100644 --- a/src/eir.c +++ b/src/eir.c @@ -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); -- 2.43.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH BlueZ v3 6/7] shared/ad: Use strtoutf8 for name 2025-07-08 15:43 [PATCH BlueZ v3 0/7] audio/avrcp: Fix crash with invalid UTF-8 item name Frédéric Danis ` (4 preceding siblings ...) 2025-07-08 15:43 ` [PATCH BlueZ v3 5/7] eir: Use strtoutf8 for device names Frédéric Danis @ 2025-07-08 15:43 ` Frédéric Danis 2025-07-08 17:12 ` Pauli Virtanen 2025-07-08 15:43 ` [PATCH BlueZ v3 7/7] unit/test-eir: Add name encoding tests Frédéric Danis 2025-07-08 16:30 ` [PATCH BlueZ v3 0/7] audio/avrcp: Fix crash with invalid UTF-8 item name patchwork-bot+bluetooth 7 siblings, 1 reply; 13+ messages in thread From: Frédéric Danis @ 2025-07-08 15:43 UTC (permalink / raw) To: linux-bluetooth Truncate the string to first character before invalid UTF-8 one instead of replacing non ascii characters by spaces. --- src/shared/ad.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/shared/ad.c b/src/shared/ad.c index 3f0064dd9..6952a0dab 100644 --- a/src/shared/ad.c +++ b/src/shared/ad.c @@ -276,7 +276,6 @@ static bool ad_replace_uuid128(struct bt_ad *ad, struct iovec *iov) static bool ad_replace_name(struct bt_ad *ad, struct iovec *iov) { char utf8_name[HCI_MAX_NAME_LENGTH + 2]; - int i; memset(utf8_name, 0, sizeof(utf8_name)); strncpy(utf8_name, (const char *)iov->iov_base, iov->iov_len); @@ -284,11 +283,7 @@ static bool ad_replace_name(struct bt_ad *ad, struct iovec *iov) if (strisutf8(utf8_name, iov->iov_len)) goto done; - /* 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, iov->iov_len); /* Remove leading and trailing whitespace characters */ strstrip(utf8_name); -- 2.43.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH BlueZ v3 6/7] shared/ad: Use strtoutf8 for name 2025-07-08 15:43 ` [PATCH BlueZ v3 6/7] shared/ad: Use strtoutf8 for name Frédéric Danis @ 2025-07-08 17:12 ` Pauli Virtanen 2025-07-08 17:16 ` Pauli Virtanen 2025-07-08 17:19 ` Luiz Augusto von Dentz 0 siblings, 2 replies; 13+ messages in thread From: Pauli Virtanen @ 2025-07-08 17:12 UTC (permalink / raw) To: Frédéric Danis, linux-bluetooth Hi, ti, 2025-07-08 kello 17:43 +0200, Frédéric Danis kirjoitti: > Truncate the string to first character before invalid UTF-8 one > instead of replacing non ascii characters by spaces. > --- > src/shared/ad.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/src/shared/ad.c b/src/shared/ad.c > index 3f0064dd9..6952a0dab 100644 > --- a/src/shared/ad.c > +++ b/src/shared/ad.c > @@ -276,7 +276,6 @@ static bool ad_replace_uuid128(struct bt_ad *ad, struct iovec *iov) > static bool ad_replace_name(struct bt_ad *ad, struct iovec *iov) > { > char utf8_name[HCI_MAX_NAME_LENGTH + 2]; > - int i; > > memset(utf8_name, 0, sizeof(utf8_name)); > strncpy(utf8_name, (const char *)iov->iov_base, iov->iov_len); > @@ -284,11 +283,7 @@ static bool ad_replace_name(struct bt_ad *ad, struct iovec *iov) > if (strisutf8(utf8_name, iov->iov_len)) > goto done; > > - /* 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, iov->iov_len); Looks like potential out-of-bounds access --- strtoutf8() may access iov->iov_base[iov->iov_len] Cf. for (size_t j = 1; j < size; ++j) loop in strtoutf8(). Also strisutf8() has same problem here. > > /* Remove leading and trailing whitespace characters */ > strstrip(utf8_name); -- Pauli Virtanen ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH BlueZ v3 6/7] shared/ad: Use strtoutf8 for name 2025-07-08 17:12 ` Pauli Virtanen @ 2025-07-08 17:16 ` Pauli Virtanen 2025-07-08 17:19 ` Luiz Augusto von Dentz 1 sibling, 0 replies; 13+ messages in thread From: Pauli Virtanen @ 2025-07-08 17:16 UTC (permalink / raw) To: Frédéric Danis, linux-bluetooth ti, 2025-07-08 kello 20:12 +0300, Pauli Virtanen kirjoitti: > Hi, > > ti, 2025-07-08 kello 17:43 +0200, Frédéric Danis kirjoitti: > > Truncate the string to first character before invalid UTF-8 one > > instead of replacing non ascii characters by spaces. > > --- > > src/shared/ad.c | 7 +------ > > 1 file changed, 1 insertion(+), 6 deletions(-) > > > > diff --git a/src/shared/ad.c b/src/shared/ad.c > > index 3f0064dd9..6952a0dab 100644 > > --- a/src/shared/ad.c > > +++ b/src/shared/ad.c > > @@ -276,7 +276,6 @@ static bool ad_replace_uuid128(struct bt_ad *ad, struct iovec *iov) > > static bool ad_replace_name(struct bt_ad *ad, struct iovec *iov) > > { > > char utf8_name[HCI_MAX_NAME_LENGTH + 2]; > > - int i; > > > > memset(utf8_name, 0, sizeof(utf8_name)); > > strncpy(utf8_name, (const char *)iov->iov_base, iov->iov_len); > > @@ -284,11 +283,7 @@ static bool ad_replace_name(struct bt_ad *ad, struct iovec *iov) > > if (strisutf8(utf8_name, iov->iov_len)) > > goto done; > > > > - /* 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, iov->iov_len); > > Looks like potential out-of-bounds access --- strtoutf8() > may access iov->iov_base[iov->iov_len] Sorry, never mind --- it's using the temporary buffer here which is maybe large enough. > > Cf. for (size_t j = 1; j < size; ++j) loop in strtoutf8(). > > Also strisutf8() has same problem here. > > > > > /* Remove leading and trailing whitespace characters */ > > strstrip(utf8_name); ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH BlueZ v3 6/7] shared/ad: Use strtoutf8 for name 2025-07-08 17:12 ` Pauli Virtanen 2025-07-08 17:16 ` Pauli Virtanen @ 2025-07-08 17:19 ` Luiz Augusto von Dentz 1 sibling, 0 replies; 13+ messages in thread From: Luiz Augusto von Dentz @ 2025-07-08 17:19 UTC (permalink / raw) To: Pauli Virtanen; +Cc: Frédéric Danis, linux-bluetooth Hi Pauli, On Tue, Jul 8, 2025 at 1:13 PM Pauli Virtanen <pav@iki.fi> wrote: > > Hi, > > ti, 2025-07-08 kello 17:43 +0200, Frédéric Danis kirjoitti: > > Truncate the string to first character before invalid UTF-8 one > > instead of replacing non ascii characters by spaces. > > --- > > src/shared/ad.c | 7 +------ > > 1 file changed, 1 insertion(+), 6 deletions(-) > > > > diff --git a/src/shared/ad.c b/src/shared/ad.c > > index 3f0064dd9..6952a0dab 100644 > > --- a/src/shared/ad.c > > +++ b/src/shared/ad.c > > @@ -276,7 +276,6 @@ static bool ad_replace_uuid128(struct bt_ad *ad, struct iovec *iov) > > static bool ad_replace_name(struct bt_ad *ad, struct iovec *iov) > > { > > char utf8_name[HCI_MAX_NAME_LENGTH + 2]; > > - int i; > > > > memset(utf8_name, 0, sizeof(utf8_name)); > > strncpy(utf8_name, (const char *)iov->iov_base, iov->iov_len); > > @@ -284,11 +283,7 @@ static bool ad_replace_name(struct bt_ad *ad, struct iovec *iov) > > if (strisutf8(utf8_name, iov->iov_len)) > > goto done; > > > > - /* 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, iov->iov_len); > > Looks like potential out-of-bounds access --- strtoutf8() > may access iov->iov_base[iov->iov_len] > > Cf. for (size_t j = 1; j < size; ++j) loop in strtoutf8(). > > Also strisutf8() has same problem here. It does i < len though: while (i < len) { unsigned char c = str[i]; That said we may need to do something like: diff --git a/src/shared/util.c b/src/shared/util.c index 4780f26b6d59..9ba1bdc48f77 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -1987,7 +1987,7 @@ char *strtoutf8(char *str, size_t len) * format. */ for (size_t j = 1; j < size; ++j) { - if (i + j > len || (str[i + j] & 0xC0) != 0x80) + if (i + j >= len || (str[i + j] & 0xC0) != 0x80) /* Invalid UTF-8 sequence */ goto done; } Otherwise we may access str[len] which is past the bondaries of str. > > > > > /* Remove leading and trailing whitespace characters */ > > strstrip(utf8_name); > > -- > Pauli Virtanen > -- Luiz Augusto von Dentz ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH BlueZ v3 7/7] unit/test-eir: Add name encoding tests 2025-07-08 15:43 [PATCH BlueZ v3 0/7] audio/avrcp: Fix crash with invalid UTF-8 item name Frédéric Danis ` (5 preceding siblings ...) 2025-07-08 15:43 ` [PATCH BlueZ v3 6/7] shared/ad: Use strtoutf8 for name Frédéric Danis @ 2025-07-08 15:43 ` Frédéric Danis 2025-07-08 16:30 ` [PATCH BlueZ v3 0/7] audio/avrcp: Fix crash with invalid UTF-8 item name patchwork-bot+bluetooth 7 siblings, 0 replies; 13+ messages in thread From: Frédéric Danis @ 2025-07-08 15:43 UTC (permalink / raw) To: linux-bluetooth This ensures that device name encode with UTF-16, ISO-2022-JP or with an incorrect character in UTF-8 string are truncated correctly. --- unit/test-eir.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/unit/test-eir.c b/unit/test-eir.c index 49ce65f24..b94a2985a 100644 --- a/unit/test-eir.c +++ b/unit/test-eir.c @@ -396,6 +396,50 @@ static const struct test_data fuelband_test = { .uuid = fuelband_uuid, }; +static const unsigned char invalid_utf8_name_data[] = { + 0x22, 0x09, 0x74, 0x65, 0x73, 0x74, 0x20, 0xe0, + 0xa4, 0xaa, 0xe0, 0xa4, 0xb0, 0xe0, 0xa5, 0x80, + 0xe0, 0xa4, /*0x95,*/ 0xe0, 0xa5, 0x8d, 0xe0, 0xa4, + 0xb7, 0xe0, 0xa4, 0xbe, 0x20, 0x69, 0x6e, 0x76, + 0x61, 0x6c, 0x69, 0x64, +}; + +static const struct test_data invalid_utf8_name_test = { + .eir_data = invalid_utf8_name_data, + .eir_size = sizeof(invalid_utf8_name_data), + .name = "test परी", + .name_complete = true, + .tx_power = 127, +}; + +static const unsigned char utf16_name_data[] = { + 0x17, 0x09, 0x00, 0x55, 0x00, 0x54, 0x00, 0x46, + 0x00, 0x2d, 0x00, 0x31, 0x00, 0x36, 0x00, 0x20, + 0x00, 0x74, 0x00, 0x65, 0x00, 0x73, 0x00, 0x74, +}; + +static const struct test_data utf16_name_test = { + .eir_data = utf16_name_data, + .eir_size = sizeof(utf16_name_data), + .name = "", + .name_complete = true, + .tx_power = 127, +}; + +static const unsigned char iso_2022_jp_name_data[] = { + 0x13, 0x09, 0x74, 0x65, 0x73, 0x74, 0x20, 0x1B, + 0x24, 0x42, 0xbb, 0xfa, 0xb8, 0xb5, 0x1b, 0x28, + 0x42, 0x20, 0x4f, 0x4b, +}; + +static const struct test_data iso_2022_jp_name_test = { + .eir_data = iso_2022_jp_name_data, + .eir_size = sizeof(iso_2022_jp_name_data), + .name = "test \033$B", + .name_complete = true, + .tx_power = 127, +}; + static const unsigned char bluesc_data[] = { 0x02, 0x01, 0x06, 0x03, 0x02, 0x16, 0x18, 0x12, 0x09, 0x57, 0x61, 0x68, 0x6f, 0x6f, 0x20, 0x42, @@ -707,6 +751,12 @@ int main(int argc, char *argv[]) tester_add("/eir/sl910", &gigaset_sl910_test, NULL, test_parsing, NULL); tester_add("/eir/bh907", &nokia_bh907_test, NULL, test_parsing, NULL); tester_add("/eir/fuelband", &fuelband_test, NULL, test_parsing, NULL); + tester_add("/eir/invalid-utf8-name", &invalid_utf8_name_test, NULL, + test_parsing, NULL); + tester_add("/eir/utf16-name", &utf16_name_test, NULL, test_parsing, + NULL); + tester_add("/eir/iso-2022-jp-name", &iso_2022_jp_name_test, NULL, + test_parsing, NULL); tester_add("/ad/bluesc", &bluesc_test, NULL, test_parsing, NULL); tester_add("/ad/wahooscale", &wahoo_scale_test, NULL, test_parsing, NULL); -- 2.43.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH BlueZ v3 0/7] audio/avrcp: Fix crash with invalid UTF-8 item name 2025-07-08 15:43 [PATCH BlueZ v3 0/7] audio/avrcp: Fix crash with invalid UTF-8 item name Frédéric Danis ` (6 preceding siblings ...) 2025-07-08 15:43 ` [PATCH BlueZ v3 7/7] unit/test-eir: Add name encoding tests Frédéric Danis @ 2025-07-08 16:30 ` patchwork-bot+bluetooth 7 siblings, 0 replies; 13+ messages in thread From: patchwork-bot+bluetooth @ 2025-07-08 16:30 UTC (permalink / raw) To: =?utf-8?b?RnLDqWTDqXJpYyBEYW5pcyA8ZnJlZGVyaWMuZGFuaXNAY29sbGFib3JhLmNvbT4=?= Cc: linux-bluetooth Hello: This series was applied to bluetooth/bluez.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Tue, 8 Jul 2025 17:43:00 +0200 you wrote: > 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. > > [...] Here is the summary with links: - [BlueZ,v3,1/7] shared/util: Add strtoutf8 function https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=0440ca68a8c4 - [BlueZ,v3,2/7] audio/avrcp: Fix crash with invalid UTF-8 item name https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=89079f2ca635 - [BlueZ,v3,3/7] audio/mcp: Use strtoutf8 for player name and track title https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=a8f59849253d - [BlueZ,v3,4/7] audio/gap: Use strtoutf8 for GAP device name https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=a0387d66372b - [BlueZ,v3,5/7] eir: Use strtoutf8 for device names https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=218db700f493 - [BlueZ,v3,6/7] shared/ad: Use strtoutf8 for name https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=4cedd8d19ad5 - [BlueZ,v3,7/7] unit/test-eir: Add name encoding tests https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=004518d061d4 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-07-08 17:19 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-08 15:43 [PATCH BlueZ v3 0/7] audio/avrcp: Fix crash with invalid UTF-8 item name Frédéric Danis 2025-07-08 15:43 ` [PATCH BlueZ v3 1/7] shared/util: Add strtoutf8 function Frédéric Danis 2025-07-08 17:06 ` audio/avrcp: Fix crash with invalid UTF-8 item name bluez.test.bot 2025-07-08 15:43 ` [PATCH BlueZ v3 2/7] " Frédéric Danis 2025-07-08 15:43 ` [PATCH BlueZ v3 3/7] audio/mcp: Use strtoutf8 for player name and track title Frédéric Danis 2025-07-08 15:43 ` [PATCH BlueZ v3 4/7] audio/gap: Use strtoutf8 for GAP device name Frédéric Danis 2025-07-08 15:43 ` [PATCH BlueZ v3 5/7] eir: Use strtoutf8 for device names Frédéric Danis 2025-07-08 15:43 ` [PATCH BlueZ v3 6/7] shared/ad: Use strtoutf8 for name Frédéric Danis 2025-07-08 17:12 ` Pauli Virtanen 2025-07-08 17:16 ` Pauli Virtanen 2025-07-08 17:19 ` Luiz Augusto von Dentz 2025-07-08 15:43 ` [PATCH BlueZ v3 7/7] unit/test-eir: Add name encoding tests Frédéric Danis 2025-07-08 16:30 ` [PATCH BlueZ v3 0/7] audio/avrcp: Fix crash with invalid UTF-8 item name patchwork-bot+bluetooth
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox