All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ v2 00/10] Replace the name2utf8 copies with str2utf8
@ 2026-08-20 18:30 Luiz Augusto von Dentz
  2026-08-20 18:30 ` [PATCH BlueZ v2 01/10] eir: Fix stack buffer overflow when parsing the remote name Luiz Augusto von Dentz
                   ` (9 more replies)
  0 siblings, 10 replies; 13+ messages in thread
From: Luiz Augusto von Dentz @ 2026-08-20 18:30 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

There are five near copies of the same helper turning a remote name into
a UTF-8 string, in monitor/att.c, profiles/audio/mcp.c,
profiles/gap/gas.c, src/eir.c and src/shared/ad.c, and none of them
agree with each other.

The copy in src/eir.c does not clamp the length before copying into a
fixed size stack buffer, and eir_parse() can hand it 253 bytes for a 250
byte buffer, so a remote device can overflow it with a long name in an
advertising report. That is fixed first and on its own so it can be
backported.

The rest replaces the copies with a single str2utf8() in src/shared/util
and drops around 120 lines.

Three things change behaviour and are worth a look:

- Most of the copies truncated the name at the first ill-formed
  sequence, throwing the rest away, and the monitor replaced every
  non-ASCII byte with a space, mangling the valid part of the name.
  str2utf8() replaces only the ill-formed sequences with U+FFFD, so a
  name is no longer cut short by one bad byte in the middle. The
  unit/test-eir expectations are updated to match.

- strnlenutf8(), and with it strisutf8() and strtoutf8(), only checked
  the shape of the bytes, so it accepted overlong encodings, UTF-16
  surrogates and code points past U+10FFFF. Those reach D-Bus, which
  does validate UTF-8 and rejects them. It now validates as per table
  3-7 of the Unicode Standard.

- btd_device_set_name() cut the name at 248 bytes with strncpy(),
  without regard for where the characters start and end, so a longer
  name could be left with a partial sequence and be rejected by D-Bus.
  It now truncates on a character boundary. It also loses the doubled
  "device" in its name.

str2utf8() and strtoutf8() were checked against Python's UTF-8 decoder
over every one and two byte sequence, a sample of the three byte ones
and 200000 random inputs, with no mismatch.

Changes in v2:

- Add unit/test-eir tests for the longest local names eir_parse() can be
  handed. Nothing covered a name anywhere near the size of the buffer it
  is copied into, which is why the missing clamp went unnoticed. Run
  against the code before the first patch, the 253 byte one dies with
  "*** buffer overflow detected ***".

- Add a fix for ad_replace_name() reading past the name it copied, which
  those new tests turned up. It clamps the copy to HCI_MAX_NAME_LENGTH
  but then hands the unclamped length to strisutf8() and strtoutf8(), so
  a 253 byte name has both reading 3 bytes past the same buffer.

- Clamp the name in eir_parse() to HCI_MAX_NAME_LENGTH rather than to
  the size of the buffer, so that it agrees with ad_replace_name() and
  the new tests can expect the same name out of both.

- Add the Fixes tag to the first patch.

Luiz Augusto von Dentz (10):
  eir: Fix stack buffer overflow when parsing the remote name
  shared/ad: Fix reading past the name that was copied
  unit/test-eir: Add tests for the longest local names
  shared/util: Make strnlenutf8 reject ill-formed sequences
  shared/util: Add str2utf8
  unit/test-util: Add str2utf8 tests
  Replace the name2utf8 copies with str2utf8
  device: Fix the name truncation splitting UTF-8 sequences
  device: Rename btd_device_device_set_name to btd_device_set_name
  unit/test-util: Cover strtoutf8 with the str2utf8 tests

 monitor/att.c        |  66 +++++++--------------
 plugins/neard.c      |   2 +-
 plugins/sixaxis.c    |   2 +-
 profiles/audio/mcp.c |  24 +-------
 profiles/gap/gas.c   |  22 +------
 src/adapter.c        |   4 +-
 src/device.c         |  14 ++++-
 src/device.h         |   2 +-
 src/eir.c            |  20 +------
 src/shared/ad.c      |  20 +++----
 src/shared/util.c    | 136 ++++++++++++++++++++++++++++++++++---------
 src/shared/util.h    |   7 +++
 unit/test-eir.c      |  69 +++++++++++++++++++++-
 unit/test-util.c     | 121 ++++++++++++++++++++++++++++++++++++++
 14 files changed, 356 insertions(+), 153 deletions(-)

-- 
2.54.0


^ permalink raw reply	[flat|nested] 13+ messages in thread
* [PATCH BlueZ v1 1/8] eir: Fix stack buffer overflow when parsing the remote name
@ 2026-08-19 20:40 Luiz Augusto von Dentz
  2026-08-19 21:27 ` Replace the name2utf8 copies with str2utf8 bluez.test.bot
  0 siblings, 1 reply; 13+ messages in thread
From: Luiz Augusto von Dentz @ 2026-08-19 20:40 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

name2utf8() copies len bytes into a HCI_MAX_NAME_LENGTH + 2, so 250,
byte stack buffer without clamping len first.

eir_parse() only rejects a field once it runs past the end of the EIR
data, and that data is up to 255 bytes, so field_len can be 254 and the
data_len passed to name2utf8() can reach 253. strncpy() then writes 253
bytes into the 250 byte buffer and leaves it unterminated, so the
following g_strstrip() and g_strdup() also read past the end.

The EIR data comes from a remote device, either in an extended inquiry
response or in an advertising report, so the length is attacker
controlled.

Clamp len like the other name2utf8() copies already do. Parsing a 253
byte EIR_NAME_COMPLETE field returned a 253 byte name before this
change, and returns a 249 byte one after it.

Assisted-by: Claude:claude-opus-5
---
 src/eir.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/eir.c b/src/eir.c
index 89c15995a546..95351d015323 100644
--- a/src/eir.c
+++ b/src/eir.c
@@ -137,6 +137,8 @@ static char *name2utf8(const uint8_t *name, uint8_t len)
 {
 	char utf8_name[HCI_MAX_NAME_LENGTH + 2];
 
+	len = MIN(len, sizeof(utf8_name) - 1);
+
 	memset(utf8_name, 0, sizeof(utf8_name));
 	strncpy(utf8_name, (char *) name, len);
 	strtoutf8(utf8_name, len);
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-08-20 20:04 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-20 18:30 [PATCH BlueZ v2 00/10] Replace the name2utf8 copies with str2utf8 Luiz Augusto von Dentz
2026-08-20 18:30 ` [PATCH BlueZ v2 01/10] eir: Fix stack buffer overflow when parsing the remote name Luiz Augusto von Dentz
2026-08-20 20:04   ` Replace the name2utf8 copies with str2utf8 bluez.test.bot
2026-08-20 18:30 ` [PATCH BlueZ v2 02/10] shared/ad: Fix reading past the name that was copied Luiz Augusto von Dentz
2026-08-20 18:30 ` [PATCH BlueZ v2 03/10] unit/test-eir: Add tests for the longest local names Luiz Augusto von Dentz
2026-08-20 18:30 ` [PATCH BlueZ v2 04/10] shared/util: Make strnlenutf8 reject ill-formed sequences Luiz Augusto von Dentz
2026-08-20 18:30 ` [PATCH BlueZ v2 05/10] shared/util: Add str2utf8 Luiz Augusto von Dentz
2026-08-20 18:30 ` [PATCH BlueZ v2 06/10] unit/test-util: Add str2utf8 tests Luiz Augusto von Dentz
2026-08-20 18:30 ` [PATCH BlueZ v2 07/10] Replace the name2utf8 copies with str2utf8 Luiz Augusto von Dentz
2026-08-20 18:30 ` [PATCH BlueZ v2 08/10] device: Fix the name truncation splitting UTF-8 sequences Luiz Augusto von Dentz
2026-08-20 18:30 ` [PATCH BlueZ v2 09/10] device: Rename btd_device_device_set_name to btd_device_set_name Luiz Augusto von Dentz
2026-08-20 18:30 ` [PATCH BlueZ v2 10/10] unit/test-util: Cover strtoutf8 with the str2utf8 tests Luiz Augusto von Dentz
  -- strict thread matches above, loose matches on Subject: below --
2026-08-19 20:40 [PATCH BlueZ v1 1/8] eir: Fix stack buffer overflow when parsing the remote name Luiz Augusto von Dentz
2026-08-19 21:27 ` Replace the name2utf8 copies with str2utf8 bluez.test.bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.