Fix writing past a buffer in checkUnusedValues()

After upgrading rrdtool from 1.9.0 to 1.10.0, some RRD-Simple-1.44
tests started to crash. An example with t/21synopsis.t:

    $ valgrind -- perl -Ilib t/21synopsis.t
    ==3822== Memcheck, a memory error detector
    ==3822== Copyright (C) 2002-2026, and GNU GPL'd, by Julian Seward et al.
    ==3822== Using Valgrind-3.27.1 and LibVEX; rerun with -h for copyright info
    ==3822== Command: perl -Ilib t/21synopsis.t
    ==3822==
    1..7
    ok 1 - new
    ok 2 - create
    ok 3 - update
    ok 4 - last
    ok 5 - sources
    *** buffer overflow detected ***: terminated
    ==3822==
    ==3822== Process terminating with default action of signal 6 (SIGABRT): dumping core
    ==3822==    at 0x4D27E9C: __pthread_kill_implementation (pthread_kill.c:44)
    ==3822==    by 0x4CCCECD: raise (raise.c:26)
    ==3822==    by 0x4CB4432: abort (abort.c:77)
    ==3822==    by 0x4CB5483: __libc_message_impl.cold (libc_fatal.c:138)
    ==3822==    by 0x4DB564F: __libc_message_wrapper (stdio.h:203)
    ==3822==    by 0x4DB564F: __fortify_fail (fortify_fail.c:24)
    ==3822==    by 0x4DB4FE3: __chk_fail (chk_fail.c:28)
    ==3822==    by 0x4DB6ABA: __strcat_chk (strcat_chk.c:34)
    ==3822==    by 0x66D0C56: strcat (string_fortified.h:152)
    ==3822==    by 0x66D0C56: checkUnusedValues (rrd_graph_helper.c:148)
    ==3822==    by 0x66D0C56: rrd_graph_script (rrd_graph_helper.c:2075)
    ==3822==    by 0x66C3A41: rrd_graph_v (rrd_graph.c:4740)
    ==3822==    by 0x66C823A: rrd_graph (rrd_graph.c:4635)
    ==3822==    by 0x668B86A: XS_RRDs_graph (RRDs.xs:407)
    ==3822==    by 0x494DB80: Perl_rpp_invoke_xs (inline.h:1176)
    ==3822==    by 0x494DB80: Perl_pp_entersub (pp_hot.c:6558)

It looks that strcat() did not like its arguments. checkUnusedValues() did:

    (gdb) frame 8
    #8  checkUnusedValues (pa=0x7fffffffbe00) at /usr/src/debug/rrdtool-1.10.0-2.fc45.x86_64/src/rrd_graph_helper.c:148
    148                 strcat(res, ":");

After reading checkUnusedValues() code, I believe that it is a bug in
that function:

"res" variable is allocated to exactly accommodate pa->kv_args[i].keyvalue (see
"res = malloc(len);" where len is one byte more than strlen() of keyvalue
string), that is a content of keyvalue and a trailing null byte. Then the
content including the trailing null is copied there with strncat(). And
finally, strcat() is called to append ":". This call writes to the last byte of
"res" buffer the colon and then writes a trailing null byte past the buffer.
This probably triggers the abort.

This patch fixes the buffer overflow by initializing the "len"
variable to 1, i.e. to count the trailing null byte.

After applying this fix the RRD-Simple-1.44 tests do not crash with
the glibc SIGABORT anymore. Instead they fail with:

    Unused Arguments " Tue 26/May/2026 12:08:05 CEST\r:" in command : COMMENT:Graph last updated: Tue 26/May/2026 12:08:05 CEST\r at t/21synopsis.t line 49.

But that's a different problem.

diff --git a/src/rrd_graph_helper.c b/src/rrd_graph_helper.c
index 987999d297b3af4a4bcefc743aa4da796f5ae8af..02034727cf504234646794dff2978432f112a989 100644 (file)
--- a/src/rrd_graph_helper.c
+++ b/src/rrd_graph_helper.c
@@ -120,7 +120,7 @@ char     *checkUnusedValues(
     parsedargs_t *pa)
 {
     char     *res = NULL;
-    size_t    len = 0;
+    size_t    len = 1;
 
     for (int i = 0; i < pa->kv_cnt; i++) {
         if (!pa->kv_args[i].flag) {