]> git.ipfire.org Git - thirdparty/libsolv.git/commitdiff
Fix a possible format overflow in dump_genid() 571/head
authorPetr Písař <ppisar@redhat.com>
Wed, 10 Jul 2024 14:54:56 +0000 (16:54 +0200)
committerPetr Písař <ppisar@redhat.com>
Wed, 10 Jul 2024 15:35:25 +0000 (17:35 +0200)
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

index d3533b9bff24de3071c4e6a9a1126c574c6471da..1640ad7ad7586d943b65d5361636dcd9508eb0cf 100644 (file)
@@ -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))