[PATCH BlueZ v1 0/8] Replace the name2utf8 copies with str2utf8
Luiz Augusto von Dentz <[email protected]>
| Newsgroups | org.kernel.vger.linux-bluetooth |
|---|---|
| Message-ID | <[email protected]> |
From: Luiz Augusto von Dentz <[email protected]> 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. Luiz Augusto von Dentz (8): eir: Fix stack buffer overflow when parsing the remote name 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 | 7 ++- unit/test-util.c | 121 ++++++++++++++++++++++++++++++++++++++ 14 files changed, 294 insertions(+), 153 deletions(-) -- 2.54.0