From 661ecff45c5f4c41c22ef9fd4fe308100b97d6bf Mon Sep 17 00:00:00 2001 From: Richard Laager Date: Fri, 11 Jul 2025 18:19:31 -0500 Subject: check_ssh: Fix buffer overflow A buffer overflow was occurring when the server responded with: Exceeded MaxStartups\r\n glibc would then abort() with the following output: *** buffer overflow detected ***: terminated It was the memset() that was overflowing the buffer. But the memmove() needed fixing too. First off, there was an off-by-one error in both the memmove() and memset(). byte_offset was already set to the start of the data _past_ the newline (i.e. len + 1). For the memmove(), incrementing that by 1 again lost the first character of the additional output. For the memset(), this causes a buffer overflow. Second, the memset() has multiple issues. The comment claims that it was NULing (sic "null") the "rest". However, it has no idea how long the "rest" is, at this point. It was NULing BUFF_SZ - byte_offset + 1. After fixing the off-by-one / buffer overflow, it would be NULing BUFF_SZ - byte_offset. But that doesn't make any sense. The length of the first line has no relation to the length of the second line. For a quick-and-dirty test, add something like this just inside the while loop: memcpy(output, "Exceeded MaxStartups\r\nnext blah1 blah2 blah3 blah4\0", sizeof("Exceeded MaxStartups\r\nnext blah1 blah2 blah3 blah4\0")); And, after the memmove(), add: printf("output='%s'\n", output); If you fix the memset() buffer overflow, it will output: output='ext blah1 blah2 blah3 ' As you can see, the first character is lost. If you then fix the memmove(), it will output: output='next blah1 blah2 blah3' Note that this is still losing the "blah4". After moving the memset() after byte_offset is set to the new strlen() of output, then it works correctly: output='next blah1 blah2 blah3 blah4' Signed-off-by: Richard Laager --- plugins/check_ssh.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'plugins/check_ssh.c') diff --git a/plugins/check_ssh.c b/plugins/check_ssh.c index 9d0d7cde..fd082e11 100644 --- a/plugins/check_ssh.c +++ b/plugins/check_ssh.c @@ -273,12 +273,14 @@ int ssh_connect(mp_check *overall, char *haddr, int hport, char *desired_remote_ } if (version_control_string == NULL) { - /* move unconsumed data to beginning of buffer, null rest */ - memmove((void *)output, (void *)(output + byte_offset + 1), BUFF_SZ - len + 1); - memset(output + byte_offset + 1, 0, BUFF_SZ - byte_offset + 1); + /* move unconsumed data to beginning of buffer */ + memmove((void *)output, (void *)(output + byte_offset), BUFF_SZ - byte_offset); /*start reading from end of current line chunk on next recv*/ byte_offset = strlen(output); + + /* NUL the rest of the buffer */ + memset(output + byte_offset, 0, BUFF_SZ - byte_offset); } } else { byte_offset += recv_ret; -- cgit v1.2.3-74-g34f1 From 1f2acfd1c6577db6e3d385614922e32ac9fad03f Mon Sep 17 00:00:00 2001 From: Richard Laager Date: Fri, 11 Jul 2025 18:38:42 -0500 Subject: check_ssh: Correct type on len variable strlen() returns a size_t. Signed-off-by: Richard Laager --- plugins/check_ssh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'plugins/check_ssh.c') diff --git a/plugins/check_ssh.c b/plugins/check_ssh.c index fd082e11..2c76fa84 100644 --- a/plugins/check_ssh.c +++ b/plugins/check_ssh.c @@ -255,7 +255,7 @@ int ssh_connect(mp_check *overall, char *haddr, int hport, char *desired_remote_ byte_offset = 0; char *index = NULL; - unsigned long len = 0; + size_t len = 0; while ((index = strchr(output + byte_offset, '\n')) != NULL) { /*Partition the buffer so that this line is a separate string, * by replacing the newline with NUL*/ -- cgit v1.2.3-74-g34f1 From 69925c782bf70d267ea14a57d46b5390f5555b8f Mon Sep 17 00:00:00 2001 From: Lorenz Kästle <12514511+RincewindsHat@users.noreply.github.com> Date: Fri, 1 Aug 2025 14:34:01 +0200 Subject: check_ssh: fix data type to allow for error checking --- plugins/check_ssh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'plugins/check_ssh.c') diff --git a/plugins/check_ssh.c b/plugins/check_ssh.c index 2c76fa84..f93127ce 100644 --- a/plugins/check_ssh.c +++ b/plugins/check_ssh.c @@ -245,7 +245,7 @@ int ssh_connect(mp_check *overall, char *haddr, int hport, char *desired_remote_ char *output = (char *)calloc(BUFF_SZ + 1, sizeof(char)); char *buffer = NULL; - size_t recv_ret = 0; + ssize_t recv_ret = 0; char *version_control_string = NULL; size_t byte_offset = 0; while ((version_control_string == NULL) && @@ -283,7 +283,7 @@ int ssh_connect(mp_check *overall, char *haddr, int hport, char *desired_remote_ memset(output + byte_offset, 0, BUFF_SZ - byte_offset); } } else { - byte_offset += recv_ret; + byte_offset += (size_t)recv_ret; } } -- cgit v1.2.3-74-g34f1 From 3c53bf623d89650ac450be2518d17276a29247cc Mon Sep 17 00:00:00 2001 From: Lorenz Kästle <12514511+RincewindsHat@users.noreply.github.com> Date: Fri, 1 Aug 2025 14:34:29 +0200 Subject: check_ssh: Fix format expression --- plugins/check_ssh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'plugins/check_ssh.c') diff --git a/plugins/check_ssh.c b/plugins/check_ssh.c index f93127ce..af7089a7 100644 --- a/plugins/check_ssh.c +++ b/plugins/check_ssh.c @@ -289,7 +289,7 @@ int ssh_connect(mp_check *overall, char *haddr, int hport, char *desired_remote_ if (recv_ret < 0) { connection_sc = mp_set_subcheck_state(connection_sc, STATE_CRITICAL); - xasprintf(&connection_sc.output, "%s", "SSH CRITICAL - %s", strerror(errno)); + xasprintf(&connection_sc.output, "%s - %s", "SSH CRITICAL - ", strerror(errno)); mp_add_subcheck_to_check(overall, connection_sc); return OK; } -- cgit v1.2.3-74-g34f1 From a69dff15222ad43c56f0142e20d97ee51c2e6697 Mon Sep 17 00:00:00 2001 From: Lorenz Kästle <12514511+RincewindsHat@users.noreply.github.com> Date: Fri, 1 Aug 2025 14:35:13 +0200 Subject: check_ssh: Put variable in the correct scope --- plugins/check_ssh.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'plugins/check_ssh.c') diff --git a/plugins/check_ssh.c b/plugins/check_ssh.c index af7089a7..b4a98cdf 100644 --- a/plugins/check_ssh.c +++ b/plugins/check_ssh.c @@ -255,12 +255,11 @@ int ssh_connect(mp_check *overall, char *haddr, int hport, char *desired_remote_ byte_offset = 0; char *index = NULL; - size_t len = 0; while ((index = strchr(output + byte_offset, '\n')) != NULL) { /*Partition the buffer so that this line is a separate string, * by replacing the newline with NUL*/ output[(index - output)] = '\0'; - len = strlen(output + byte_offset); + size_t len = strlen(output + byte_offset); if ((len >= 4) && (strncmp(output + byte_offset, "SSH-", 4) == 0)) { /*if the string starts with SSH-, this _should_ be a valid version control string*/ -- cgit v1.2.3-74-g34f1 From 2757550558d509aa5c95d8834ee76d803e110161 Mon Sep 17 00:00:00 2001 From: Lorenz Kästle <12514511+RincewindsHat@users.noreply.github.com> Date: Fri, 1 Aug 2025 14:35:23 +0200 Subject: clang-format --- plugins/check_ssh.c | 72 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 29 deletions(-) (limited to 'plugins/check_ssh.c') diff --git a/plugins/check_ssh.c b/plugins/check_ssh.c index b4a98cdf..f6c8d551 100644 --- a/plugins/check_ssh.c +++ b/plugins/check_ssh.c @@ -57,7 +57,8 @@ static process_arguments_wrapper process_arguments(int /*argc*/, char ** /*argv* static void print_help(void); void print_usage(void); -static int ssh_connect(mp_check *overall, char *haddr, int hport, char *remote_version, char *remote_protocol); +static int ssh_connect(mp_check *overall, char *haddr, int hport, char *remote_version, + char *remote_protocol); int main(int argc, char **argv) { setlocale(LC_ALL, ""); @@ -85,7 +86,8 @@ int main(int argc, char **argv) { alarm(socket_timeout); /* ssh_connect exits if error is found */ - ssh_connect(&overall, config.server_name, config.port, config.remote_version, config.remote_protocol); + ssh_connect(&overall, config.server_name, config.port, config.remote_version, + config.remote_protocol); alarm(0); @@ -96,19 +98,20 @@ int main(int argc, char **argv) { /* process command-line arguments */ process_arguments_wrapper process_arguments(int argc, char **argv) { - static struct option longopts[] = {{"help", no_argument, 0, 'h'}, - {"version", no_argument, 0, 'V'}, - {"host", required_argument, 0, 'H'}, /* backward compatibility */ - {"hostname", required_argument, 0, 'H'}, - {"port", required_argument, 0, 'p'}, - {"use-ipv4", no_argument, 0, '4'}, - {"use-ipv6", no_argument, 0, '6'}, - {"timeout", required_argument, 0, 't'}, - {"verbose", no_argument, 0, 'v'}, - {"remote-version", required_argument, 0, 'r'}, - {"remote-protocol", required_argument, 0, 'P'}, - {"output-format", required_argument, 0, output_format_index}, - {0, 0, 0, 0}}; + static struct option longopts[] = { + {"help", no_argument, 0, 'h'}, + {"version", no_argument, 0, 'V'}, + {"host", required_argument, 0, 'H'}, /* backward compatibility */ + {"hostname", required_argument, 0, 'H'}, + {"port", required_argument, 0, 'p'}, + {"use-ipv4", no_argument, 0, '4'}, + {"use-ipv6", no_argument, 0, '6'}, + {"timeout", required_argument, 0, 't'}, + {"verbose", no_argument, 0, 'v'}, + {"remote-version", required_argument, 0, 'r'}, + {"remote-protocol", required_argument, 0, 'P'}, + {"output-format", required_argument, 0, output_format_index}, + {0, 0, 0, 0}}; process_arguments_wrapper result = { .config = check_ssh_config_init(), @@ -228,7 +231,8 @@ process_arguments_wrapper process_arguments(int argc, char **argv) { * *-----------------------------------------------------------------------*/ -int ssh_connect(mp_check *overall, char *haddr, int hport, char *desired_remote_version, char *desired_remote_protocol) { +int ssh_connect(mp_check *overall, char *haddr, int hport, char *desired_remote_version, + char *desired_remote_protocol) { struct timeval tv; gettimeofday(&tv, NULL); @@ -238,7 +242,8 @@ int ssh_connect(mp_check *overall, char *haddr, int hport, char *desired_remote_ mp_subcheck connection_sc = mp_subcheck_init(); if (result != STATE_OK) { connection_sc = mp_set_subcheck_state(connection_sc, STATE_CRITICAL); - xasprintf(&connection_sc.output, "Failed to establish TCP connection to Host %s and Port %d", haddr, hport); + xasprintf(&connection_sc.output, + "Failed to establish TCP connection to Host %s and Port %d", haddr, hport); mp_add_subcheck_to_check(overall, connection_sc); return result; } @@ -249,7 +254,8 @@ int ssh_connect(mp_check *overall, char *haddr, int hport, char *desired_remote_ char *version_control_string = NULL; size_t byte_offset = 0; while ((version_control_string == NULL) && - (recv_ret = recv(socket, output + byte_offset, (unsigned long)(BUFF_SZ - byte_offset), 0) > 0)) { + (recv_ret = recv(socket, output + byte_offset, (unsigned long)(BUFF_SZ - byte_offset), + 0) > 0)) { if (strchr(output, '\n')) { /* we've got at least one full line, start parsing*/ byte_offset = 0; @@ -262,7 +268,8 @@ int ssh_connect(mp_check *overall, char *haddr, int hport, char *desired_remote_ size_t len = strlen(output + byte_offset); if ((len >= 4) && (strncmp(output + byte_offset, "SSH-", 4) == 0)) { - /*if the string starts with SSH-, this _should_ be a valid version control string*/ + /*if the string starts with SSH-, this _should_ be a valid version control + * string*/ version_control_string = output + byte_offset; break; } @@ -334,7 +341,8 @@ int ssh_connect(mp_check *overall, char *haddr, int hport, char *desired_remote_ * "1.x" (e.g., "1.5" or "1.3")." * - RFC 4253:5 */ - char *ssh_server = ssh_proto + strspn(ssh_proto, "0123456789.") + 1; /* (+1 for the '-' separating protoversion from softwareversion) */ + char *ssh_server = ssh_proto + strspn(ssh_proto, "0123456789.") + + 1; /* (+1 for the '-' separating protoversion from softwareversion) */ /* If there's a space in the version string, whatever's after the space is a comment * (which is NOT part of the server name/version)*/ @@ -346,13 +354,15 @@ int ssh_connect(mp_check *overall, char *haddr, int hport, char *desired_remote_ mp_subcheck protocol_validity_sc = mp_subcheck_init(); if (strlen(ssh_proto) == 0 || strlen(ssh_server) == 0) { protocol_validity_sc = mp_set_subcheck_state(protocol_validity_sc, STATE_CRITICAL); - xasprintf(&protocol_validity_sc.output, "Invalid protocol version control string %s", version_control_string); + xasprintf(&protocol_validity_sc.output, "Invalid protocol version control string %s", + version_control_string); mp_add_subcheck_to_check(overall, protocol_validity_sc); return OK; } protocol_validity_sc = mp_set_subcheck_state(protocol_validity_sc, STATE_OK); - xasprintf(&protocol_validity_sc.output, "Valid protocol version control string %s", version_control_string); + xasprintf(&protocol_validity_sc.output, "Valid protocol version control string %s", + version_control_string); mp_add_subcheck_to_check(overall, protocol_validity_sc); ssh_proto[strspn(ssh_proto, "0123456789. ")] = 0; @@ -367,8 +377,8 @@ int ssh_connect(mp_check *overall, char *haddr, int hport, char *desired_remote_ if (desired_remote_version && strcmp(desired_remote_version, ssh_server)) { mp_subcheck remote_version_sc = mp_subcheck_init(); remote_version_sc = mp_set_subcheck_state(remote_version_sc, STATE_CRITICAL); - xasprintf(&remote_version_sc.output, _("%s (protocol %s) version mismatch, expected '%s'"), ssh_server, ssh_proto, - desired_remote_version); + xasprintf(&remote_version_sc.output, _("%s (protocol %s) version mismatch, expected '%s'"), + ssh_server, ssh_proto, desired_remote_version); close(socket); mp_add_subcheck_to_check(overall, remote_version_sc); return OK; @@ -386,11 +396,13 @@ int ssh_connect(mp_check *overall, char *haddr, int hport, char *desired_remote_ if (desired_remote_protocol && strcmp(desired_remote_protocol, ssh_proto)) { protocol_version_sc = mp_set_subcheck_state(protocol_version_sc, STATE_CRITICAL); - xasprintf(&protocol_version_sc.output, _("%s (protocol %s) protocol version mismatch, expected '%s'"), ssh_server, ssh_proto, - desired_remote_protocol); + xasprintf(&protocol_version_sc.output, + _("%s (protocol %s) protocol version mismatch, expected '%s'"), ssh_server, + ssh_proto, desired_remote_protocol); } else { protocol_version_sc = mp_set_subcheck_state(protocol_version_sc, STATE_OK); - xasprintf(&protocol_version_sc.output, "SSH server version: %s (protocol version: %s)", ssh_server, ssh_proto); + xasprintf(&protocol_version_sc.output, "SSH server version: %s (protocol version: %s)", + ssh_server, ssh_proto); } mp_add_subcheck_to_check(overall, protocol_version_sc); @@ -423,7 +435,8 @@ void print_help(void) { printf(UT_CONN_TIMEOUT, DEFAULT_SOCKET_TIMEOUT); printf(" %s\n", "-r, --remote-version=STRING"); - printf(" %s\n", _("Alert if string doesn't match expected server version (ex: OpenSSH_3.9p1)")); + printf(" %s\n", + _("Alert if string doesn't match expected server version (ex: OpenSSH_3.9p1)")); printf(" %s\n", "-P, --remote-protocol=STRING"); printf(" %s\n", _("Alert if protocol doesn't match expected protocol version (ex: 2.0)")); @@ -436,5 +449,6 @@ void print_help(void) { void print_usage(void) { printf("%s\n", _("Usage:")); - printf("%s [-4|-6] [-t ] [-r ] [-p ] --hostname \n", progname); + printf("%s [-4|-6] [-t ] [-r ] [-p ] --hostname \n", + progname); } -- cgit v1.2.3-74-g34f1