From f31c6de1a7051d2d98efec2a56b92f6da0fef537 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= Date: Wed, 10 Jul 2024 16:54:56 +0200 Subject: [PATCH] Fix a possible format overflow in dump_genid() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GCC 14 called with CFLAGS='-O2 -Wformat-overflow' complains: /tmp/libsolv/ext/testcase.c: In function ‘dump_genid’: /tmp/libsolv/ext/testcase.c:1275:33: warning: ‘: genid ’ directive writing 8 bytes into a region of size between 3 and 12 [-Wformat-overflow=] 1275 | sprintf(cntbuf, "genid %2d: genid ", cnt++); | ^~~~~~~~ /tmp/libsolv/ext/testcase.c:1275:7: note: ‘sprintf’ output between 17 and 26 bytes into a destination of size 20 1275 | sprintf(cntbuf, "genid %2d: genid ", cnt++); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /tmp/libsolv/ext/testcase.c:1270:33: warning: ‘: genid ’ directive writing 8 bytes into a region of size between 3 and 12 [-Wformat-overflow=] 1270 | sprintf(cntbuf, "genid %2d: genid ", cnt++); | ^~~~~~~~ /tmp/libsolv/ext/testcase.c:1270:7: note: ‘sprintf’ output between 17 and 26 bytes into a destination of size 20 1270 | sprintf(cntbuf, "genid %2d: genid ", cnt++); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ That's indeed a bug: sprintf() writes into a 20-byte array cntbuf. cnt is int, 32-bit long integer on x86_64 Linux platform. dump_genid() starts with cnt = 1 and increases. It can go up to 2147483647 decimal value, then wrap to -2147483648 decimal value. That's up to 11 bytes of the integer, plus 14 bytes of a static string, plus 1 byte of a trailing '\0'. 26 bytes in total. While it's improbable that cnt would amount that long number in real life, it's better to be prepared for the worst. Also a benefit is that static analyzers will be be content. This patch increases cntbuf[] size to accomodate common 32-bit ints. (Generic, albeit illegible, expression would be: cntbuf[((sizeof(cnt) * 8 - 1) * 3 / 10 + 1 + 1) + 14 + 1]; but I'm not sure that long expression is worth of it.) --- ext/testcase.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/testcase.c b/ext/testcase.c index d3533b9bf..1640ad7ad 100644 --- a/ext/testcase.c +++ b/ext/testcase.c @@ -1256,7 +1256,7 @@ static int dump_genid(Pool *pool, Strqueue *sq, Id id, int cnt) { struct oplist *op; - char cntbuf[20]; + char cntbuf[26]; const char *s; if (ISRELDEP(id))