Fix possible buffer overrun due to incorrect strncat length argument

The strncat usage assumed that the length argument to strncat
indicates the length of the destination buffer. That is how strlcat
works. The length argument for strncat instead describes the maximum
number of characters to copy from the source buffer.

To make the call sites work correctly when we want to avoid
overflowing the destination buffer, we need to subtract also the
current length of the destination buffer string.

This also cures possible overflow issues with any of the strncat use
sites.

Signed-off-by: Timo Kokkonen <timo.t.kokkonen@iki.fi>

diff --git a/plugin_manager.c b/plugin_manager.c
index 6ddac4cda4bf3ff3130a21bccd24e333af54ed3b..e91698090613eaf696dabaecbbd764bda15b56e4 100644 (file)
--- a/plugin_manager.c
+++ b/plugin_manager.c
@@ -80,14 +80,16 @@ int load_parser_plugin(const char *name)
        int ret;
 
        strncpy(str, name, sizeof(str));
-       strncat(str, parser, sizeof(str) - 1);
+       str[sizeof(str) - 1] = '\0';
+       strncat(str, parser, sizeof(str) - strlen(str) - 1);
        ret = load_plugin(str);
        if (!ret)
                return 0;
 
        strncpy(str, "./", sizeof(str));
-       strncat(str, name, sizeof(str) - 1);
-       strncat(str, parser, sizeof(str) - 1);
+       str[sizeof(str) - 1] = '\0';
+       strncat(str, name, sizeof(str) - strlen(str) - 1);
+       strncat(str, parser, sizeof(str) - strlen(str) - 1);
        ret = load_plugin(str);
        if (!ret)
                return 0;
@@ -96,8 +98,9 @@ int load_parser_plugin(const char *name)
                return 0;
 
        strncpy(str, exec_path, sizeof(str));
-       strncat(str, "/", sizeof(str) - 1);
-       strncat(str, name, sizeof(str) - 1);
-       strncat(str, parser, sizeof(str) - 1);
+       str[sizeof(str) - 1] = '\0';
+       strncat(str, "/", sizeof(str) - strlen(str) - 1);
+       strncat(str, name, sizeof(str) - strlen(str) - 1);
+       strncat(str, parser, sizeof(str) - strlen(str) - 1);
        return load_plugin(str);
 }

diff --git a/rrdtool.c b/rrdtool.c
index da876d51da7620d8051ca6f0320790c71f5e687f..c92dee186c56c9a28951d10aa089ab47f0816419 100644 (file)
--- a/rrdtool.c
+++ b/rrdtool.c
@@ -47,8 +47,8 @@ int rrdtool_draw_image(struct rrd_image *image)
        pr_info("Drawing image %s\n", image->image_filename);
 
        tmpfile[0] = 0;
-       strncat(tmpfile, image->image_filename, sizeof(tmp) - 1);
-       strncat(tmpfile, ".tmp", sizeof(tmp) - 1);
+       strncat(tmpfile, image->image_filename, sizeof(tmp) - strlen(tmp) - 1);
+       strncat(tmpfile, ".tmp", sizeof(tmp) - strlen(tmp) - 1);
 
        if (image->updatestr)
                updatestr = image->updatestr;