From f1f186534e252c72aff958ea5db4f387b075d73e Mon Sep 17 00:00:00 2001 From: Timo Kokkonen Date: Tue, 15 Nov 2016 19:44:47 +0200 Subject: [PATCH] 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 --- plugin_manager.c | 15 +++++++++------ rrdtool.c | 4 ++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/plugin_manager.c b/plugin_manager.c index 6ddac4c..e916980 100644 --- 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 da876d5..c92dee1 100644 --- 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; -- 2.45.0