ALT Linux Sisyphus

From b7bf4ff64187616ac21895cd016d62017ac3b681 Mon Sep 17 00:00:00 2001
From: Alexey Sheplyakov <asheplyakov@basealt.ru>
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=<optimized out>, flags@entry=32, subject=subject@entry=0x7fffffffd5b0,
    predicate=predicate@entry=0x7fffffffd670, type_urid=<optimized out>, size=3, body=0x7fffffffe288) at ../src/sratom.c:338
#9  0x000055555555ecf0 in sratom_write (sratom=sratom@entry=0x55555556a930, unmap=unmap@entry=0x7fffffffdce0, flags=<optimized out>, 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=<optimized out>, flags@entry=34, subject=subject@entry=0x7fffffffd9d0,
    predicate=predicate@entry=0x7fffffffda90, type_urid=<optimized out>, size=56, body=0x7fffffffe270) at ../src/sratom.c:432
#12 0x000055555555e60b in sratom_write (sratom=sratom@entry=0x55555556a930, unmap=unmap@entry=0x7fffffffdce0, flags=<optimized out>, 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