From e1b93f596067106e7c29c263b8bb4d0b51718d9d Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Mon, 29 Jun 2015 17:32:44 +0200 Subject: [PATCH] src/daemon/utils_subst.c: Sanitize buffer offsets better. The previous implementation broke when off1 / off2 were outside of "string" but within "buflen". It also had problems if the replacement string was too long. This new implementation truncates the buffer as expected and is properly tested. --- src/daemon/Makefile.am | 8 +++-- src/daemon/utils_subst.c | 75 +++++++++++++++++++++++++--------------- src/target_replace.c | 2 +- 3 files changed, 54 insertions(+), 31 deletions(-) diff --git a/src/daemon/Makefile.am b/src/daemon/Makefile.am index 00a8fd73..8275eee8 100644 --- a/src/daemon/Makefile.am +++ b/src/daemon/Makefile.am @@ -89,8 +89,8 @@ else collectd_LDADD += -loconfig endif -check_PROGRAMS = test_common test_utils_avltree test_utils_heap -TESTS = test_common test_utils_avltree test_utils_heap +check_PROGRAMS = test_common test_utils_avltree test_utils_heap test_utils_subst +TESTS = test_common test_utils_avltree test_utils_heap test_utils_subst test_common_SOURCES = common_test.c ../testing.h test_common_LDADD = libcommon.la libplugin_mock.la $(COMMON_LIBS) @@ -100,3 +100,7 @@ test_utils_avltree_LDADD = libavltree.la $(COMMON_LIBS) test_utils_heap_SOURCES = utils_heap_test.c ../testing.h test_utils_heap_LDADD = libheap.la $(COMMON_LIBS) + +test_utils_subst_SOURCES = utils_subst_test.c ../testing.h \ + utils_subst.c utils_subst.h +test_utils_subst_LDADD = libcommon.la libplugin_mock.la $(COMMON_LIBS) diff --git a/src/daemon/utils_subst.c b/src/daemon/utils_subst.c index 2f28eb9b..43634df2 100644 --- a/src/daemon/utils_subst.c +++ b/src/daemon/utils_subst.c @@ -31,33 +31,60 @@ #include "collectd.h" #include "common.h" -char *subst (char *buf, size_t buflen, const char *string, int off1, int off2, +char *subst (char *buf, size_t buflen, const char *string, size_t off1, size_t off2, const char *replacement) { - char *buf_ptr = buf; - size_t len = buflen; + char *out = buf; - if ((NULL == buf) || (0 >= buflen) || (NULL == string) - || (0 > off1) || (0 > off2) || (off1 > off2) - || (NULL == replacement)) + char const *front; + char const *back; + size_t front_len; + size_t replacement_len; + size_t back_len; + + if ((NULL == buf) || (0 == buflen) || (NULL == string) || (NULL == replacement)) return NULL; - sstrncpy (buf_ptr, string, - ((size_t)off1 + 1 > buflen) ? buflen : (size_t)off1 + 1); - buf_ptr += off1; - len -= off1; + size_t string_len = strlen (string); + if ((off1 > string_len) || (off2 > string_len) || (off1 > off2)) + return NULL; - if (0 >= len) - return buf; + front = string; + back = string + off2; + front_len = off1; + replacement_len = strlen (replacement); + back_len = strlen (back); + + if (front_len >= buflen) { + front_len = buflen - 1; + replacement_len = 0; + back_len = 0; + } else if ((front_len + replacement_len) >= buflen) { + replacement_len = buflen - (front_len + 1); + back_len = 0; + } else if ((front_len + replacement_len + back_len) >= buflen) { + back_len = buflen - (front_len + replacement_len + 1); + } else { + buflen = front_len + replacement_len + back_len + 1; + } + assert ((front_len + replacement_len + back_len) == (buflen - 1)); - sstrncpy (buf_ptr, replacement, len); - buf_ptr += strlen (replacement); - len -= strlen (replacement); + if (front_len != 0) { + sstrncpy (out, front, front_len + 1); + out += front_len; + } - if (0 >= len) - return buf; + if (replacement_len != 0) { + sstrncpy (out, replacement, replacement_len + 1); + out += replacement_len; + } - sstrncpy (buf_ptr, string + off2, len); + if (back_len != 0) { + sstrncpy (out, back, back_len + 1); + out += back_len; + } + + out[0] = 0; return buf; } /* subst */ @@ -87,7 +114,6 @@ char *asubst (const char *string, int off1, int off2, const char *replacement) char *subst_string (char *buf, size_t buflen, const char *string, const char *needle, const char *replacement) { - char *temp; size_t needle_len; size_t i; @@ -95,19 +121,13 @@ char *subst_string (char *buf, size_t buflen, const char *string, || (needle == NULL) || (replacement == NULL)) return (NULL); - temp = (char *) malloc (buflen); - if (temp == NULL) - { - ERROR ("subst_string: malloc failed."); - return (NULL); - } - needle_len = strlen (needle); - strncpy (buf, string, buflen); + sstrncpy (buf, string, buflen); /* Limit the loop to prevent endless loops. */ for (i = 0; i < buflen; i++) { + char temp[buflen]; char *begin_ptr; size_t begin; @@ -141,7 +161,6 @@ char *subst_string (char *buf, size_t buflen, const char *string, i, string, needle, replacement); } - sfree (temp); return (buf); } /* char *subst_string */ diff --git a/src/target_replace.c b/src/target_replace.c index a85eced3..bd8f9e5c 100644 --- a/src/target_replace.c +++ b/src/target_replace.c @@ -190,7 +190,7 @@ static int tr_action_invoke (tr_action_t *act_head, /* {{{ */ } subst_status = subst (temp, sizeof (temp), buffer, - matches[0].rm_so, matches[0].rm_eo, act->replacement); + (size_t) matches[0].rm_so, (size_t) matches[0].rm_eo, act->replacement); if (subst_status == NULL) { ERROR ("Target `replace': subst (buffer = %s, start = %zu, end = %zu, " -- 2.30.2