Re: [PATCH BlueZ] uuid: fix 1 byte stack overflow

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



AddressSanitizer is part of newer gcc & clang versions.

All I did was build bluez with:

make CFLAGS=-fsanitize=address\ -fsanitize=undefined\ -ggdb3\ -Wall\
-Wextra\ -Wno-unused-parameter\ -Wno-missing-field-initializers\
-Werror LDFLAGS=-pthread -j10

The LDFLAGS bit is to work around a bug in binutils, and the warnings
could be omitted if desired. Debug info is needed for address
sanitizer to give line numbers.


On Tue, Mar 8, 2016 at 9:20 AM, Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
> Hi Cody,
>
> On Mon, Mar 7, 2016 at 11:01 PM, Cody P Schafer <dev@xxxxxxxxxx> wrote:
>> scanf requires that '[' convertion specifiers have enough room for all
>> characters in the string, _plus a terminating null byte_. We were
>> previously not providing room for the terminating null byte.
>>
>> This was detected by AddressSanitizer:
>>
>> ==15036==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe4e774401 at pc 0x7fd33f572c98 bp 0x7ffe4e774270 sp 0x7ffe4e7739f8
>> WRITE of size 2 at 0x7ffe4e774401 thread T0
>>     #0 0x7fd33f572c97 in scanf_common /build/gcc-multilib/src/gcc-5-20160209/libsanitizer/sanitizer_common/sanitizer_common_interceptors_format.inc:340
>>     #1 0x7fd33f5739ea in __interceptor_vsscanf /build/gcc-multilib/src/gcc-5-20160209/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:769
>>     #2 0x7fd33f573b49 in __interceptor_sscanf /build/gcc-multilib/src/gcc-5-20160209/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:793
>>     #3 0x650db5 in is_base_uuid128 lib/uuid.c:191
>>     #4 0x65196e in bt_string_to_uuid lib/uuid.c:267
>>     #5 0x56f28e in parse_uuid src/gatt-database.c:1473
>>     #6 0x5729e0 in database_add_service src/gatt-database.c:2053
>>     #7 0x57329f in database_add_app src/gatt-database.c:2106
>>     #8 0x573adc in client_ready_cb src/gatt-database.c:2211
>>     #9 0x6695fd in get_managed_objects_reply gdbus/client.c:1097
>>     #10 0x7fd33efd5391  (/usr/lib/libdbus-1.so.3+0x13391)
>>     #11 0x7fd33efd8db0 in dbus_connection_dispatch (/usr/lib/libdbus-1.so.3+0x16db0)
>>     #12 0x651ecd in message_dispatch gdbus/mainloop.c:72
>>     #13 0x7fd33f25cc39 in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x49c39)
>>     #14 0x7fd33f25cfdf  (/usr/lib/libglib-2.0.so.0+0x49fdf)
>>     #15 0x7fd33f25d301 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x4a301)
>>     #16 0x54b7d1 in main src/main.c:687
>>     #17 0x7fd33d90870f in __libc_start_main (/usr/lib/libc.so.6+0x2070f)
>>     #18 0x40bba8 in _start (/home/cody/g/bluez/src/bluetoothd+0x40bba8)
>>
>> Address 0x7ffe4e774401 is located in stack of thread T0 at offset 33 in frame
>>     #0 0x650ccd in is_base_uuid128 lib/uuid.c:184
>>
>>   This frame has 2 object(s):
>>     [32, 33) 'dummy' <== Memory access at offset 33 overflows this variable
>>     [96, 98) 'uuid'
>> HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
>>       (longjmp and C++ exceptions *are* supported)
>> SUMMARY: AddressSanitizer: stack-buffer-overflow /build/gcc-multilib/src/gcc-5-20160209/libsanitizer/sanitizer_common/sanitizer_common_interceptors_format.inc:340 scanf_common
>> Shadow bytes around the buggy address:
>>   0x100049ce6830: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>   0x100049ce6840: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>   0x100049ce6850: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>   0x100049ce6860: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>   0x100049ce6870: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
>> =>0x100049ce6880:[01]f4 f4 f4 f2 f2 f2 f2 02 f4 f4 f4 f3 f3 f3 f3
>>   0x100049ce6890: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
>>   0x100049ce68a0: 00 f4 f4 f4 f2 f2 f2 f2 00 00 04 f4 f2 f2 f2 f2
>>   0x100049ce68b0: 00 00 00 00 00 00 00 00 00 f4 f4 f4 f3 f3 f3 f3
>>   0x100049ce68c0: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
>>   0x100049ce68d0: 01 f4 f4 f4 f2 f2 f2 f2 00 00 04 f4 f3 f3 f3 f3
>> Shadow byte legend (one shadow byte represents 8 application bytes):
>>   Addressable:           00
>>   Partially addressable: 01 02 03 04 05 06 07
>>   Heap left redzone:       fa
>>   Heap right redzone:      fb
>>   Freed heap region:       fd
>>   Stack left redzone:      f1
>>   Stack mid redzone:       f2
>>   Stack right redzone:     f3
>>   Stack partial redzone:   f4
>>   Stack after return:      f5
>>   Stack use after scope:   f8
>>   Global redzone:          f9
>>   Global init order:       f6
>>   Poisoned by user:        f7
>>   Container overflow:      fc
>>   Array cookie:            ac
>>   Intra object redzone:    bb
>>   ASan internal:           fe
>> ==15036==ABORTING
>> ---
>>  lib/uuid.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/uuid.c b/lib/uuid.c
>> index 20b67d0..ac071fa 100644
>> --- a/lib/uuid.c
>> +++ b/lib/uuid.c
>> @@ -183,14 +183,14 @@ static inline int is_uuid128(const char *string)
>>  static inline int is_base_uuid128(const char *string)
>>  {
>>         uint16_t uuid;
>> -       char dummy;
>> +       char dummy[2];
>>
>>         if (!is_uuid128(string))
>>                 return 0;
>>
>>         return sscanf(string,
>>                 "0000%04hx-0000-1000-8000-00805%1[fF]9%1[bB]34%1[fF]%1[bB]",
>> -               &uuid, &dummy, &dummy, &dummy, &dummy) == 5;
>> +               &uuid, dummy, dummy, dummy, dummy) == 5;
>>  }
>>
>>  static inline int is_uuid32(const char *string)
>> --
>> 2.7.2
>
> Interesting it seems this has been broken for a while although
> valgrind could not detect this invalid access, or perhaps we don't
> really execute this code with unit tests. Anyway we could possible add
> this tool to be run with make check but I could not find any match for
> AddressSanitizer in fedora at least.
>
> --
> Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux