From b7bf4ff64187616ac21895cd016d62017ac3b681 Mon Sep 17 00:00:00 2001 From: Alexey Sheplyakov Date: Fri, 14 Jul 2023 15:14:15 +0400 Subject: [PATCH] Fixed sratom_test false negative At the moment sratom_test fails like this: $ ./sratom_test *** buffer overflow detected ***: terminated Aborted (gdb) bt #0 0x00007ffff7e365dc in ?? () from /lib64/libc.so.6 #1 0x00007ffff7de8d32 in raise () from /lib64/libc.so.6 #2 0x00007ffff7dd24af in abort () from /lib64/libc.so.6 #3 0x00007ffff7dd322f in ?? () from /lib64/libc.so.6 #4 0x00007ffff7ec5a87 in __fortify_fail () from /lib64/libc.so.6 #5 0x00007ffff7ec4422 in __chk_fail () from /lib64/libc.so.6 #6 0x00007ffff7ec3ff5 in __snprintf_chk () from /lib64/libc.so.6 #7 0x000055555555ea63 in snprintf (__fmt=0x555555563bfb "%02X", __n=7, __s=0x55555556d9f2 "") at /usr/include/bits/stdio2.h:54 #8 sratom_write (sratom=sratom@entry=0x55555556a930, unmap=unmap@entry=0x7fffffffdce0, flags=, flags@entry=32, subject=subject@entry=0x7fffffffd5b0, predicate=predicate@entry=0x7fffffffd670, type_urid=, size=3, body=0x7fffffffe288) at ../src/sratom.c:338 #9 0x000055555555ecf0 in sratom_write (sratom=sratom@entry=0x55555556a930, unmap=unmap@entry=0x7fffffffdce0, flags=, subject=subject@entry=0x7fffffffd830, predicate=predicate@entry=0x7fffffffd8d0, type_urid=type_urid@entry=19, size=19, body=0x7fffffffe278) at ../src/sratom.c:362 #10 0x000055555555f71c in list_append (sratom=sratom@entry=0x55555556a930, unmap=unmap@entry=0x7fffffffdce0, flags=flags@entry=0x7fffffffd80c, s=s@entry=0x7fffffffd810, p=p@entry=0x7fffffffd8d0, node=node@entry=0x7fffffffd830, size=19, type=19, body=0x7fffffffe278) at ../src/sratom.c:172 #11 0x000055555555f4a5 in sratom_write (sratom=sratom@entry=0x55555556a930, unmap=unmap@entry=0x7fffffffdce0, flags=, flags@entry=34, subject=subject@entry=0x7fffffffd9d0, predicate=predicate@entry=0x7fffffffda90, type_urid=, size=56, body=0x7fffffffe270) at ../src/sratom.c:432 #12 0x000055555555e60b in sratom_write (sratom=sratom@entry=0x55555556a930, unmap=unmap@entry=0x7fffffffdce0, flags=, flags@entry=2, subject=subject@entry=0x7fffffffdd70, predicate=predicate@entry=0x7fffffffdd90, type_urid=type_urid@entry=9, size=1000, body=0x7fffffffdf08) at ../src/sratom.c:417 #13 0x000055555555f92c in sratom_to_turtle (sratom=sratom@entry=0x55555556a930, unmap=unmap@entry=0x7fffffffdce0, base_uri=base_uri@entry=0x555555563383 "file:///tmp/base/", subject=subject@entry=0x7fffffffdd70, predicate=predicate@entry=0x7fffffffdd90, type=9, size=1000, body=0x7fffffffdf08) at ../src/sratom.c:496 #14 0x000055555555a6f7 in test (env=env@entry=0x0, top_level=top_level@entry=false, pretty_numbers=pretty_numbers@entry=false) at ../tests/sratom_test.c:339 #15 0x000055555555a875 in test_env (env=env@entry=0x0) at ../tests/sratom_test.c:386 #16 0x00005555555585ba in main () at ../tests/sratom_test.c:403 (gdb) frame 7 #7 0x000055555555ea63 in snprintf (__fmt=0x555555563bfb "%02X", __n=7, __s=0x55555556d9f2 "") at /usr/include/bits/stdio2.h:54 54 return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1, (gdb) list 49 # ifdef __va_arg_pack 50 __fortify_function int 51 __NTH (snprintf (char *__restrict __s, size_t __n, 52 const char *__restrict __fmt, ...)) 53 { 54 return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1, 55 __glibc_objsize (__s), __fmt, 56 __va_arg_pack ()); 57 } 58 # elif !defined __cplusplus The code in question (sratom.c:338, see frame 9) dumps the input byte by byte (in hex format): 333 } else if (type_urid == sratom->midi_MidiEvent) { 334 new_node = true; 335 datatype = serd_node_from_string(SERD_URI, USTR(LV2_MIDI__MidiEvent)); 336 uint8_t* str = (uint8_t*)calloc(size * 2 + 1, 1); 337 for (uint32_t i = 0; i < size; ++i) { 338 snprintf((char*)str + (2 * i), size * 2 + 1, "%02X", 339 (unsigned)*((const uint8_t*)body + i)); 340 } The problem is that the maximal number of bytes in sprintf is overestimated (when i != 0): actually snprintf prints at most 3 bytes (2 hex digits and possibly the terminating null), instead the code specifies the total length of the dumped buffer (`size * 2 + 1`). When i > 0 glibc's __snprintf_chk observes the destination buffer is shorter than the `size * 2 + 1` (the total length of the dump) and terminates the program. To avoid the problem specify the correct n, which is 3 (2 hex digits and possibly the terminating null). --- src/sratom.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/sratom.c b/src/sratom.c index 248906f..9cec78a 100644 --- a/src/sratom.c +++ b/src/sratom.c @@ -335,7 +335,9 @@ sratom_write(Sratom* sratom, datatype = serd_node_from_string(SERD_URI, USTR(LV2_MIDI__MidiEvent)); uint8_t* str = (uint8_t*)calloc(size * 2 + 1, 1); for (uint32_t i = 0; i < size; ++i) { - snprintf((char*)str + (2 * i), size * 2 + 1, "%02X", + snprintf((char*)str + (2 * i), + 3, /* 2 hex digits and possibly terminating null */ + "%02X", (unsigned)*((const uint8_t*)body + i)); } object = serd_node_from_string(SERD_LITERAL, USTR(str)); -- 2.33.8