From 9c7ba0d42dee5e3f84ecb6e4fcdbefc6c1cd965c Mon Sep 17 00:00:00 2001 From: Paul Mackerras Date: Mon, 19 Jul 2021 17:41:09 +1000 Subject: [PATCH] chat: Clean up usage of clean() function In a couple of places, we were calling clean(), which does environment variable substitution among other things, but then using the original string not the "cleaned" string when logging a message about what we're doing. Also, this removes a couple of checks that the "cleaned" string is not longer than the original string, which date back to the first version of the code checked into CVS. Those checks were appropriate before environment variable substitution was added in commit eaca954c2d4a ("add -E option to use environment variables, from Andreas Arens") and dynamic reallocation of the result buffer was added in commit 86dd2eec100d ("clean(): Fix buffer overflow.") but are no longer necessary. These changes were prompted by github issue #294 and redhat bugzilla https://bugzilla.redhat.com/show_bug.cgi?id=1650539 Signed-off-by: Paul Mackerras --- chat/chat.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/chat/chat.c b/chat/chat.c index 1639b3e..d2d255a 100644 --- a/chat/chat.c +++ b/chat/chat.c @@ -1020,14 +1020,13 @@ void chat_send (register char *s) s1 = clean(s, 0); - if (strlen(s1) > strlen(s) - || strlen(s1) + 1 > sizeof(fail_buffer)) + if (strlen(s1) + 1 > sizeof(fail_buffer)) fatal(1, "Illegal or too-long ABORT string ('%v')", s); abort_string[n_aborts++] = s1; if (verbose) - msgf("abort on (%v)", s); + msgf("abort on (%v)", s1); return; } @@ -1041,8 +1040,7 @@ void chat_send (register char *s) s1 = clean(s, 0); - if (strlen(s1) > strlen(s) - || strlen(s1) + 1 > sizeof(fail_buffer)) + if (strlen(s1) + 1 > sizeof(fail_buffer)) fatal(1, "Illegal or too-long CLR_ABORT string ('%v')", s); old_max = n_aborts; @@ -1053,7 +1051,7 @@ void chat_send (register char *s) pack++; n_aborts--; if (verbose) - msgf("clear abort on (%v)", s); + msgf("clear abort on (%v)", s1); } } free(s1); @@ -1070,14 +1068,13 @@ void chat_send (register char *s) fatal(2, "Too many REPORT strings"); s1 = clean(s, 0); - if (strlen(s1) > strlen(s) - || strlen(s1) + 1 > sizeof(fail_buffer)) + if (strlen(s1) + 1 > sizeof(fail_buffer)) fatal(1, "Illegal or too-long REPORT string ('%v')", s); report_string[n_reports++] = s1; if (verbose) - msgf("report (%v)", s); + msgf("report (%v)", s1); return; } @@ -1091,8 +1088,7 @@ void chat_send (register char *s) s1 = clean(s, 0); - if (strlen(s1) > strlen(s) - || strlen(s1) + 1 > sizeof(fail_buffer)) + if (strlen(s1) + 1 > sizeof(fail_buffer)) fatal(1, "Illegal or too-long REPORT string ('%v')", s); old_max = n_reports; @@ -1103,7 +1099,7 @@ void chat_send (register char *s) pack++; n_reports--; if (verbose) - msgf("clear report (%v)", s); + msgf("clear report (%v)", s1); } } free(s1); -- 2.39.2