From c2c56bacab00766d01671413321d564227aabf19 Mon Sep 17 00:00:00 2001 From: Norbert Pocs Date: Sun, 5 Nov 2023 13:12:47 +0100 Subject: [PATCH 01/12] CVE-2023-6004: torture_config: Allow multiple '@' in usernames Signed-off-by: Norbert Pocs Reviewed-by: Andreas Schneider Reviewed-by: Jakub Jelen --- tests/unittests/torture_config.c | 56 +++++++++++++++++--------------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/tests/unittests/torture_config.c b/tests/unittests/torture_config.c index 406f1985..b7c763af 100644 --- a/tests/unittests/torture_config.c +++ b/tests/unittests/torture_config.c @@ -995,23 +995,22 @@ static void torture_config_proxyjump(void **state, assert_string_equal(session->opts.ProxyCommand, "ssh -W [%h]:%p 2620:52:0::fed"); - /* In this part, we try various other config files and strings. */ - - /* Try to create some invalid configurations */ - /* Non-numeric port */ - config = "Host bad-port\n" - "\tProxyJump jumpbox:22bad22\n"; + /* Multiple @ is allowed in second jump */ + config = "Host allowed-hostname\n" + "\tProxyJump localhost,user@principal.com@jumpbox:22\n"; if (file != NULL) { torture_write_file(file, config); } else { string = config; } torture_reset_config(session); - ssh_options_set(session, SSH_OPTIONS_HOST, "bad-port"); - _parse_config(session, file, string, SSH_ERROR); + ssh_options_set(session, SSH_OPTIONS_HOST, "allowed-hostname"); + _parse_config(session, file, string, SSH_OK); + assert_string_equal(session->opts.ProxyCommand, + "ssh -J user@principal.com@jumpbox:22 -W '[%h]:%p' localhost"); - /* Too many @ */ - config = "Host bad-hostname\n" + /* Multiple @ is allowed */ + config = "Host allowed-hostname\n" "\tProxyJump user@principal.com@jumpbox:22\n"; if (file != NULL) { torture_write_file(file, config); @@ -1019,7 +1018,24 @@ static void torture_config_proxyjump(void **state, string = config; } torture_reset_config(session); - ssh_options_set(session, SSH_OPTIONS_HOST, "bad-hostname"); + ssh_options_set(session, SSH_OPTIONS_HOST, "allowed-hostname"); + _parse_config(session, file, string, SSH_OK); + assert_string_equal(session->opts.ProxyCommand, + "ssh -l user@principal.com -p 22 -W '[%h]:%p' jumpbox"); + + /* In this part, we try various other config files and strings. */ + + /* Try to create some invalid configurations */ + /* Non-numeric port */ + config = "Host bad-port\n" + "\tProxyJump jumpbox:22bad22\n"; + if (file != NULL) { + torture_write_file(file, config); + } else { + string = config; + } + torture_reset_config(session); + ssh_options_set(session, SSH_OPTIONS_HOST, "bad-port"); _parse_config(session, file, string, SSH_ERROR); /* Braces mismatch in hostname */ @@ -1094,18 +1110,6 @@ static void torture_config_proxyjump(void **state, ssh_options_set(session, SSH_OPTIONS_HOST, "bad-port-2"); _parse_config(session, file, string, SSH_ERROR); - /* Too many @ in second jump */ - config = "Host bad-hostname\n" - "\tProxyJump localhost,user@principal.com@jumpbox:22\n"; - if (file != NULL) { - torture_write_file(file, config); - } else { - string = config; - } - torture_reset_config(session); - ssh_options_set(session, SSH_OPTIONS_HOST, "bad-hostname"); - _parse_config(session, file, string, SSH_ERROR); - /* Braces mismatch in second jump */ config = "Host mismatch\n" "\tProxyJump localhost,[::1:20\n"; -- 2.41.0 From a66b4a6eae6614d200a3625862d77565b96a7cd3 Mon Sep 17 00:00:00 2001 From: Norbert Pocs Date: Wed, 1 Nov 2023 11:24:43 +0100 Subject: [PATCH 02/12] CVE-2023-6004: config_parser: Allow multiple '@' in usernames Signed-off-by: Norbert Pocs Reviewed-by: Andreas Schneider Reviewed-by: Jakub Jelen --- src/config_parser.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config_parser.c b/src/config_parser.c index 0d988fec..cf83e2c5 100644 --- a/src/config_parser.c +++ b/src/config_parser.c @@ -180,7 +180,7 @@ int ssh_config_parse_uri(const char *tok, } /* Username part (optional) */ - endp = strchr(tok, '@'); + endp = strrchr(tok, '@'); if (endp != NULL) { /* Zero-length username is not valid */ if (tok == endp) { -- 2.41.0 From 8615c24647f773a5e04203c7459512715d698be1 Mon Sep 17 00:00:00 2001 From: Norbert Pocs Date: Tue, 31 Oct 2023 09:48:52 +0100 Subject: [PATCH 03/12] CVE-2023-6004: options: Simplify the hostname parsing in ssh_options_set Using ssh_config_parse_uri can simplify the parsing of the host parsing inside the function of ssh_options_set Signed-off-by: Norbert Pocs Reviewed-by: Andreas Schneider Reviewed-by: Jakub Jelen --- src/options.c | 40 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/src/options.c b/src/options.c index 6f2c9397..38511455 100644 --- a/src/options.c +++ b/src/options.c @@ -37,6 +37,7 @@ #include "libssh/session.h" #include "libssh/misc.h" #include "libssh/options.h" +#include "libssh/config_parser.h" #ifdef WITH_SERVER #include "libssh/server.h" #include "libssh/bind.h" @@ -515,33 +516,24 @@ int ssh_options_set(ssh_session session, enum ssh_options_e type, ssh_set_error_invalid(session); return -1; } else { - q = strdup(value); - if (q == NULL) { - ssh_set_error_oom(session); + char *username = NULL, *hostname = NULL, *port = NULL; + rc = ssh_config_parse_uri(value, &username, &hostname, &port); + if (rc != SSH_OK) { return -1; } - p = strrchr(q, '@'); - - SAFE_FREE(session->opts.host); - - if (p) { - *p = '\0'; - session->opts.host = strdup(p + 1); - if (session->opts.host == NULL) { - SAFE_FREE(q); - ssh_set_error_oom(session); - return -1; - } - + if (port != NULL) { + SAFE_FREE(username); + SAFE_FREE(hostname); + SAFE_FREE(port); + return -1; + } + if (username != NULL) { SAFE_FREE(session->opts.username); - session->opts.username = strdup(q); - SAFE_FREE(q); - if (session->opts.username == NULL) { - ssh_set_error_oom(session); - return -1; - } - } else { - session->opts.host = q; + session->opts.username = username; + } + if (hostname != NULL) { + SAFE_FREE(session->opts.host); + session->opts.host = hostname; } } break; -- 2.41.0 From c6180409677c765e6b9ae2b18a3a7a9671ac1dbe Mon Sep 17 00:00:00 2001 From: Norbert Pocs Date: Tue, 10 Oct 2023 12:44:16 +0200 Subject: [PATCH 04/12] CVE-2023-6004: misc: Add function to check allowed characters of a hostname The hostname can be a domain name or an ip address. The colon has to be allowed because of IPv6 even it is prohibited in domain names. Signed-off-by: Norbert Pocs Reviewed-by: Andreas Schneider Reviewed-by: Jakub Jelen --- include/libssh/misc.h | 3 ++ src/misc.c | 68 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/src/misc.c b/src/misc.c index 7c478a77..be6ee836 100644 --- a/src/misc.c +++ b/src/misc.c @@ -1974,4 +1976,70 @@ char *ssh_strerror(int err_num, char *buf, size_t buflen) #endif /* defined(__linux__) && defined(__GLIBC__) && defined(_GNU_SOURCE) */ } +/** + * @brief Checks syntax of a domain name + * + * The check is made based on the RFC1035 section 2.3.1 + * Allowed characters are: hyphen, period, digits (0-9) and letters (a-zA-Z) + * + * The label should be no longer than 63 characters + * The label should start with a letter and end with a letter or number + * The label in this implementation can start with a number to allow virtual + * URLs to pass. Note that this will make IPv4 addresses to pass + * this check too. + * + * @param hostname The domain name to be checked, has to be null terminated + * + * @return SSH_OK if the hostname passes syntax check + * SSH_ERROR otherwise or if hostname is NULL or empty string + */ +int ssh_check_hostname_syntax(const char *hostname) +{ + char *it = NULL, *s = NULL, *buf = NULL; + size_t it_len; + char c; + + if (hostname == NULL || strlen(hostname) == 0) { + return SSH_ERROR; + } + + /* strtok_r writes into the string, keep the input clean */ + s = strdup(hostname); + if (s == NULL) { + return SSH_ERROR; + } + + it = strtok_r(s, ".", &buf); + /* if the token has 0 length */ + if (it == NULL) { + free(s); + return SSH_ERROR; + } + do { + it_len = strlen(it); + if (it_len > ARPA_DOMAIN_MAX_LEN || + /* the first char must be a letter, but some virtual urls start + * with a number */ + isalnum(it[0]) == 0 || + isalnum(it[it_len - 1]) == 0) { + free(s); + return SSH_ERROR; + } + while (*it != '\0') { + c = *it; + /* the "." is allowed too, but tokenization removes it from the + * string */ + if (isalnum(c) == 0 && c != '-') { + free(s); + return SSH_ERROR; + } + it++; + } + } while ((it = strtok_r(NULL, ".", &buf)) != NULL); + + free(s); + + return SSH_OK; +} + /** @} */ -- 2.41.0 From 9bbb817c0c5434f03613d0783b2ef5f52235b901 Mon Sep 17 00:00:00 2001 From: Norbert Pocs Date: Tue, 10 Oct 2023 12:45:28 +0200 Subject: [PATCH 05/12] CVE-2023-6004: torture_misc: Add test for ssh_check_hostname_syntax Signed-off-by: Norbert Pocs Reviewed-by: Andreas Schneider Reviewed-by: Jakub Jelen --- tests/unittests/torture_misc.c | 73 ++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/tests/unittests/torture_misc.c b/tests/unittests/torture_misc.c index 9e346ff8..e682b6d4 100644 --- a/tests/unittests/torture_misc.c +++ b/tests/unittests/torture_misc.c @@ -760,6 +760,78 @@ static void torture_ssh_strerror(void **state) assert_non_null(out); } +static void torture_ssh_check_hostname_syntax(void **state) +{ + int rc; + (void)state; + + rc = ssh_check_hostname_syntax("duckduckgo.com"); + assert_int_equal(rc, SSH_OK); + rc = ssh_check_hostname_syntax("www.libssh.org"); + assert_int_equal(rc, SSH_OK); + rc = ssh_check_hostname_syntax("Some-Thing.com"); + assert_int_equal(rc, SSH_OK); + rc = ssh_check_hostname_syntax("amazon.a23456789012345678901234567890123456789012345678901234567890123"); + assert_int_equal(rc, SSH_OK); + rc = ssh_check_hostname_syntax("amazon.a23456789012345678901234567890123456789012345678901234567890123.a23456789012345678901234567890123456789012345678901234567890123.ok"); + assert_int_equal(rc, SSH_OK); + rc = ssh_check_hostname_syntax("amazon.a23456789012345678901234567890123456789012345678901234567890123.a23456789012345678901234567890123456789012345678901234567890123.a23456789012345678901234567890123456789012345678901234567890123"); + assert_int_equal(rc, SSH_OK); + rc = ssh_check_hostname_syntax("lavabo-inter.innocentes-manus-meas"); + assert_int_equal(rc, SSH_OK); + rc = ssh_check_hostname_syntax("localhost"); + assert_int_equal(rc, SSH_OK); + rc = ssh_check_hostname_syntax("a"); + assert_int_equal(rc, SSH_OK); + rc = ssh_check_hostname_syntax("a-0.b-b"); + assert_int_equal(rc, SSH_OK); + rc = ssh_check_hostname_syntax("libssh."); + assert_int_equal(rc, SSH_OK); + + rc = ssh_check_hostname_syntax(NULL); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax(""); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("/"); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("@"); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("["); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("`"); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("{"); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("&"); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("|"); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("\""); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("`"); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax(" "); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("*the+giant&\"rooks\".c0m"); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("!www.libssh.org"); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("--.--"); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("libssh.a234567890123456789012345678901234567890123456789012345678901234"); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("libssh.a234567890123456789012345678901234567890123456789012345678901234.a234567890123456789012345678901234567890123456789012345678901234"); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("libssh-"); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("fe80::9656:d028:8652:66b6"); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax("."); + assert_int_equal(rc, SSH_ERROR); + rc = ssh_check_hostname_syntax(".."); + assert_int_equal(rc, SSH_ERROR); +} + int torture_run_tests(void) { int rc; struct CMUnitTest tests[] = { @@ -784,6 +856,7 @@ int torture_run_tests(void) { cmocka_unit_test(torture_ssh_quote_file_name), cmocka_unit_test(torture_ssh_strreplace), cmocka_unit_test(torture_ssh_strerror), + cmocka_unit_test(torture_ssh_check_hostname_syntax), }; ssh_init(); -- 2.41.0 From 22492b69bba22b102342afc574800d354a08e405 Mon Sep 17 00:00:00 2001 From: Norbert Pocs Date: Tue, 10 Oct 2023 18:33:56 +0200 Subject: [PATCH 06/12] CVE-2023-6004: config_parser: Check for valid syntax of a hostname if it is a domain name This prevents code injection. The domain name syntax checker is based on RFC1035. Signed-off-by: Norbert Pocs Reviewed-by: Andreas Schneider Reviewed-by: Jakub Jelen --- src/config_parser.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/config_parser.c b/src/config_parser.c index cf83e2c5..b8b94611 100644 --- a/src/config_parser.c +++ b/src/config_parser.c @@ -30,6 +30,7 @@ #include "libssh/config_parser.h" #include "libssh/priv.h" +#include "libssh/misc.h" /* Returns the original string after skipping the leading whitespace * and optional quotes. @@ -167,6 +168,7 @@ int ssh_config_parse_uri(const char *tok, { char *endp = NULL; long port_n; + int rc; /* Sanitize inputs */ if (username != NULL) { @@ -224,6 +226,14 @@ int ssh_config_parse_uri(const char *tok, if (*hostname == NULL) { goto error; } + /* if not an ip, check syntax */ + rc = ssh_is_ipaddr(*hostname); + if (rc == 0) { + rc = ssh_check_hostname_syntax(*hostname); + if (rc != SSH_OK) { + goto error; + } + } } /* Skip also the closing bracket */ if (*endp == ']') { -- 2.41.0 From d7467498fd988949edde9c6384973250fd454a8b Mon Sep 17 00:00:00 2001 From: Norbert Pocs Date: Tue, 10 Oct 2023 10:28:47 +0200 Subject: [PATCH 07/12] CVE-2023-6004: torture_proxycommand: Add test for proxycommand injection Signed-off-by: Norbert Pocs Reviewed-by: Andreas Schneider Reviewed-by: Jakub Jelen --- tests/client/torture_proxycommand.c | 53 +++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tests/client/torture_proxycommand.c b/tests/client/torture_proxycommand.c index 9b8019ca..1bad4ccc 100644 --- a/tests/client/torture_proxycommand.c +++ b/tests/client/torture_proxycommand.c @@ -166,6 +166,56 @@ static void torture_options_set_proxycommand_ssh_stderr(void **state) assert_int_equal(rc & O_RDWR, O_RDWR); } +static void torture_options_proxycommand_injection(void **state) +{ + struct torture_state *s = *state; + struct passwd *pwd = NULL; + const char *malicious_host = "`echo foo > mfile`"; + const char *command = "nc %h %p"; + char *current_dir = NULL; + char *malicious_file_path = NULL; + int mfp_len; + int verbosity = torture_libssh_verbosity(); + struct stat sb; + int rc; + + pwd = getpwnam("bob"); + assert_non_null(pwd); + + rc = setuid(pwd->pw_uid); + assert_return_code(rc, errno); + + s->ssh.session = ssh_new(); + assert_non_null(s->ssh.session); + + ssh_options_set(s->ssh.session, SSH_OPTIONS_LOG_VERBOSITY, &verbosity); + // if we would be checking the rc, this should fail + ssh_options_set(s->ssh.session, SSH_OPTIONS_HOST, malicious_host); + + ssh_options_set(s->ssh.session, SSH_OPTIONS_USER, TORTURE_SSH_USER_ALICE); + + rc = ssh_options_set(s->ssh.session, SSH_OPTIONS_PROXYCOMMAND, command); + assert_int_equal(rc, 0); + rc = ssh_connect(s->ssh.session); + assert_ssh_return_code_equal(s->ssh.session, rc, SSH_ERROR); + + current_dir = torture_get_current_working_dir(); + assert_non_null(current_dir); + mfp_len = strlen(current_dir) + 6; + malicious_file_path = malloc(mfp_len); + assert_non_null(malicious_file_path); + rc = snprintf(malicious_file_path, mfp_len, + "%s/mfile", current_dir); + assert_int_equal(rc, mfp_len); + free(current_dir); + rc = stat(malicious_file_path, &sb); + assert_int_not_equal(rc, 0); + + // cleanup + remove(malicious_file_path); + free(malicious_file_path); +} + int torture_run_tests(void) { int rc; struct CMUnitTest tests[] = { @@ -181,6 +231,9 @@ int torture_run_tests(void) { cmocka_unit_test_setup_teardown(torture_options_set_proxycommand_ssh_stderr, session_setup, session_teardown), + cmocka_unit_test_setup_teardown(torture_options_proxycommand_injection, + NULL, + session_teardown), }; -- 2.41.0 From 62d3101c1f76b6891b70c50154e0e934d6b8cb57 Mon Sep 17 00:00:00 2001 From: Norbert Pocs Date: Mon, 6 Nov 2023 20:11:38 +0100 Subject: [PATCH 08/12] CVE-2023-6004: torture_misc: Add test for ssh_is_ipaddr Signed-off-by: Norbert Pocs Reviewed-by: Andreas Schneider Reviewed-by: Jakub Jelen --- tests/unittests/torture_misc.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/unittests/torture_misc.c b/tests/unittests/torture_misc.c index e682b6d4..b07392ab 100644 --- a/tests/unittests/torture_misc.c +++ b/tests/unittests/torture_misc.c @@ -832,6 +832,31 @@ static void torture_ssh_check_hostname_syntax(void **state) assert_int_equal(rc, SSH_ERROR); } +static void torture_ssh_is_ipaddr(void **state) { + int rc; + (void)state; + + rc = ssh_is_ipaddr("201.255.3.69"); + assert_int_equal(rc, 1); + rc = ssh_is_ipaddr("::1"); + assert_int_equal(rc, 1); + rc = ssh_is_ipaddr("2001:0db8:85a3:0000:0000:8a2e:0370:7334"); + assert_int_equal(rc, 1); + + rc = ssh_is_ipaddr(".."); + assert_int_equal(rc, 0); + rc = ssh_is_ipaddr(":::"); + assert_int_equal(rc, 0); + rc = ssh_is_ipaddr("1.1.1.1.1"); + assert_int_equal(rc, 0); + rc = ssh_is_ipaddr("1.1"); + assert_int_equal(rc, 0); + rc = ssh_is_ipaddr("caesar"); + assert_int_equal(rc, 0); + rc = ssh_is_ipaddr("::xa:1"); + assert_int_equal(rc, 0); +} + int torture_run_tests(void) { int rc; struct CMUnitTest tests[] = { @@ -857,6 +882,7 @@ int torture_run_tests(void) { cmocka_unit_test(torture_ssh_strreplace), cmocka_unit_test(torture_ssh_strerror), cmocka_unit_test(torture_ssh_check_hostname_syntax), + cmocka_unit_test(torture_ssh_is_ipaddr), }; ssh_init(); -- 2.41.0 From cea841d71c025f9c998b7d5fc9f2a2839df62921 Mon Sep 17 00:00:00 2001 From: Norbert Pocs Date: Tue, 28 Nov 2023 15:26:45 +0100 Subject: [PATCH 09/12] CVE-2023-6004 misc: Add ipv6 link-local check for an ip address Signed-off-by: Norbert Pocs Reviewed-by: Andreas Schneider Reviewed-by: Jakub Jelen --- src/CMakeLists.txt | 1 + src/connect.c | 2 +- src/misc.c | 44 ++++++++++++++++++++++++++++++++++++++------ 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index d6245c0d..807313b5 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -91,6 +91,7 @@ endif() if (WIN32) set(LIBSSH_LINK_LIBRARIES ${LIBSSH_LINK_LIBRARIES} + iphlpapi ws2_32 ) endif (WIN32) diff --git a/src/connect.c b/src/connect.c index 57e37e63..15cae644 100644 --- a/src/connect.c +++ b/src/connect.c @@ -136,7 +136,7 @@ static int getai(const char *host, int port, struct addrinfo **ai) #endif } - if (ssh_is_ipaddr(host)) { + if (ssh_is_ipaddr(host) == 1) { /* this is an IP address */ SSH_LOG(SSH_LOG_PACKET, "host %s matches an IP address", host); hints.ai_flags |= AI_NUMERICHOST; diff --git a/src/misc.c b/src/misc.c index be6ee836..7081f12a 100644 --- a/src/misc.c +++ b/src/misc.c @@ -32,6 +32,7 @@ #include #include #include +#include #endif /* _WIN32 */ @@ -59,6 +60,7 @@ #include #include #include +#include #ifdef HAVE_IO_H #include @@ -222,22 +224,37 @@ int ssh_is_ipaddr_v4(const char *str) int ssh_is_ipaddr(const char *str) { int rc = SOCKET_ERROR; + char *s = strdup(str); - if (strchr(str, ':')) { + if (s == NULL) { + return -1; + } + if (strchr(s, ':')) { struct sockaddr_storage ss; int sslen = sizeof(ss); + char *network_interface = strchr(s, '%'); - /* TODO link-local (IP:v6:addr%ifname). */ - rc = WSAStringToAddressA((LPSTR) str, + /* link-local (IP:v6:addr%ifname). */ + if (network_interface != NULL) { + rc = if_nametoindex(network_interface + 1); + if (rc == 0) { + free(s); + return 0; + } + *network_interface = '\0'; + } + rc = WSAStringToAddressA((LPSTR) s, AF_INET6, NULL, (struct sockaddr*)&ss, &sslen); if (rc == 0) { + free(s); return 1; } } + free(s); return ssh_is_ipaddr_v4(str); } #else /* _WIN32 */ @@ -343,17 +360,32 @@ int ssh_is_ipaddr_v4(const char *str) int ssh_is_ipaddr(const char *str) { int rc = -1; + char *s = strdup(str); - if (strchr(str, ':')) { + if (s == NULL) { + return -1; + } + if (strchr(s, ':')) { struct in6_addr dest6; + char *network_interface = strchr(s, '%'); - /* TODO link-local (IP:v6:addr%ifname). */ - rc = inet_pton(AF_INET6, str, &dest6); + /* link-local (IP:v6:addr%ifname). */ + if (network_interface != NULL) { + rc = if_nametoindex(network_interface + 1); + if (rc == 0) { + free(s); + return 0; + } + *network_interface = '\0'; + } + rc = inet_pton(AF_INET6, s, &dest6); if (rc > 0) { + free(s); return 1; } } + free(s); return ssh_is_ipaddr_v4(str); } -- 2.41.0 From 2c492ee179d5caa2718c5e768bab6e0b2b64a8b0 Mon Sep 17 00:00:00 2001 From: Norbert Pocs Date: Tue, 28 Nov 2023 15:27:31 +0100 Subject: [PATCH 10/12] CVE-2023-6004: torture_misc: Add tests for ipv6 link-local Signed-off-by: Norbert Pocs Reviewed-by: Andreas Schneider Reviewed-by: Jakub Jelen --- tests/unittests/torture_misc.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/unittests/torture_misc.c b/tests/unittests/torture_misc.c index b07392ab..77166759 100644 --- a/tests/unittests/torture_misc.c +++ b/tests/unittests/torture_misc.c @@ -17,7 +17,14 @@ #include "torture.h" #include "error.c" +#ifdef _WIN32 +#include +#else +#include +#endif + #define TORTURE_TEST_DIR "/usr/local/bin/truc/much/.." +#define TORTURE_IPV6_LOCAL_LINK "fe80::98e1:82ff:fe8d:28b3%%%s" const char template[] = "temp_dir_XXXXXX"; @@ -834,14 +841,27 @@ static void torture_ssh_check_hostname_syntax(void **state) static void torture_ssh_is_ipaddr(void **state) { int rc; + char *interf = malloc(64); + char *test_interf = malloc(128); (void)state; + assert_non_null(interf); + assert_non_null(test_interf); rc = ssh_is_ipaddr("201.255.3.69"); assert_int_equal(rc, 1); rc = ssh_is_ipaddr("::1"); assert_int_equal(rc, 1); rc = ssh_is_ipaddr("2001:0db8:85a3:0000:0000:8a2e:0370:7334"); assert_int_equal(rc, 1); + if_indextoname(1, interf); + assert_non_null(interf); + rc = sprintf(test_interf, TORTURE_IPV6_LOCAL_LINK, interf); + /* the "%%s" is not written */ + assert_int_equal(rc, strlen(interf) + strlen(TORTURE_IPV6_LOCAL_LINK) - 3); + rc = ssh_is_ipaddr(test_interf); + assert_int_equal(rc, 1); + free(interf); + free(test_interf); rc = ssh_is_ipaddr(".."); assert_int_equal(rc, 0); -- 2.41.0 From 1a02364b5107a4125ea3cb76fcdb6beabaebf3be Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Fri, 22 Dec 2023 10:32:40 +0100 Subject: [PATCH 11/12] Fix regression in IPv6 addresses in hostname parsing Signed-off-by: Jakub Jelen Reviewed-by: Andreas Schneider (cherry picked from commit 4f997aee7c7d7ea346b3e8ba505da0b7601ff318) --- include/libssh/config_parser.h | 11 ++++++++--- src/config.c | 4 ++-- src/config_parser.c | 16 +++++++++++----- src/options.c | 10 ++-------- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/include/libssh/config_parser.h b/include/libssh/config_parser.h index a7dd42a2..ca353432 100644 --- a/include/libssh/config_parser.h +++ b/include/libssh/config_parser.h @@ -30,6 +30,8 @@ #ifndef CONFIG_PARSER_H_ #define CONFIG_PARSER_H_ +#include + char *ssh_config_get_cmd(char **str); char *ssh_config_get_token(char **str); diff --git a/src/config.c b/src/config.c index c5c40125..273db7c8 100644 --- a/src/config.c +++ b/src/config.c @@ -464,7 +464,7 @@ ssh_config_parse_proxy_jump(ssh_session session, const char *s, bool do_parsing) } if (parse_entry) { /* We actually care only about the first item */ - rv = ssh_config_parse_uri(cp, &username, &hostname, &port); + rv = ssh_config_parse_uri(cp, &username, &hostname, &port, false); /* The rest of the list needs to be passed on */ if (endp != NULL) { next = strdup(endp + 1); @@ -475,7 +475,7 @@ ssh_config_parse_proxy_jump(ssh_session session, const char *s, bool do_parsing) } } else { /* The rest is just sanity-checked to avoid failures later */ - rv = ssh_config_parse_uri(cp, NULL, NULL, NULL); + rv = ssh_config_parse_uri(cp, NULL, NULL, NULL, false); } if (rv != SSH_OK) { goto out; diff --git a/src/config_parser.c b/src/config_parser.c index b8b94611..d4b2d2c3 100644 --- a/src/config_parser.c +++ b/src/config_parser.c @@ -162,9 +162,10 @@ int ssh_config_get_yesno(char **str, int notfound) } int ssh_config_parse_uri(const char *tok, - char **username, - char **hostname, - char **port) + char **username, + char **hostname, + char **port, + bool ignore_port) { char *endp = NULL; long port_n; @@ -210,12 +211,17 @@ int ssh_config_parse_uri(const char *tok, if (endp == NULL) { goto error; } - } else { - /* Hostnames or aliases expand to the last colon or to the end */ + } else if (!ignore_port) { + /* Hostnames or aliases expand to the last colon (if port is requested) + * or to the end */ endp = strrchr(tok, ':'); if (endp == NULL) { endp = strchr(tok, '\0'); } + } else { + /* If no port is requested, expand to the end of line + * (to accommodate the IPv6 addresses) */ + endp = strchr(tok, '\0'); } if (tok == endp) { /* Zero-length hostnames are not valid */ diff --git a/src/options.c b/src/options.c index 38511455..b3ecffe1 100644 --- a/src/options.c +++ b/src/options.c @@ -516,17 +516,11 @@ int ssh_options_set(ssh_session session, enum ssh_options_e type, ssh_set_error_invalid(session); return -1; } else { - char *username = NULL, *hostname = NULL, *port = NULL; - rc = ssh_config_parse_uri(value, &username, &hostname, &port); + char *username = NULL, *hostname = NULL; + rc = ssh_config_parse_uri(value, &username, &hostname, NULL, true); if (rc != SSH_OK) { return -1; } - if (port != NULL) { - SAFE_FREE(username); - SAFE_FREE(hostname); - SAFE_FREE(port); - return -1; - } if (username != NULL) { SAFE_FREE(session->opts.username); session->opts.username = username; -- 2.41.0 From 6f1b1e76bb38bc89819132e1810e4301ec9034a4 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Fri, 22 Dec 2023 09:52:18 +0100 Subject: [PATCH 12/12] tests: Increase test coverage for IPv6 address parsing as hostnames This was an issue in cockpit: https://github.com/cockpit-project/cockpit/issues/19772 Signed-off-by: Jakub Jelen Reviewed-by: Andreas Schneider (cherry picked from commit 6f6e453d7b0ad4ee6a6f6a1c96a9a6b27821410d) --- tests/unittests/torture_config.c | 49 +++++++++++++++++++++++++++++++ tests/unittests/torture_options.c | 16 ++++++++++ 2 files changed, 65 insertions(+) diff --git a/tests/unittests/torture_config.c b/tests/unittests/torture_config.c index b7c763af..26a24215 100644 --- a/tests/unittests/torture_config.c +++ b/tests/unittests/torture_config.c @@ -1850,6 +1850,53 @@ static void torture_config_make_absolute_no_sshdir(void **state) torture_config_make_absolute_int(state, 1); } +static void torture_config_parse_uri(void **state) +{ + char *username = NULL; + char *hostname = NULL; + char *port = NULL; + int rc; + + (void)state; /* unused */ + + rc = ssh_config_parse_uri("localhost", &username, &hostname, &port, false); + assert_return_code(rc, errno); + assert_null(username); + assert_string_equal(hostname, "localhost"); + SAFE_FREE(hostname); + assert_null(port); + + rc = ssh_config_parse_uri("1.2.3.4", &username, &hostname, &port, false); + assert_return_code(rc, errno); + assert_null(username); + assert_string_equal(hostname, "1.2.3.4"); + SAFE_FREE(hostname); + assert_null(port); + + rc = ssh_config_parse_uri("1.2.3.4:2222", &username, &hostname, &port, false); + assert_return_code(rc, errno); + assert_null(username); + assert_string_equal(hostname, "1.2.3.4"); + SAFE_FREE(hostname); + assert_string_equal(port, "2222"); + SAFE_FREE(port); + + rc = ssh_config_parse_uri("[1:2:3::4]:2222", &username, &hostname, &port, false); + assert_return_code(rc, errno); + assert_null(username); + assert_string_equal(hostname, "1:2:3::4"); + SAFE_FREE(hostname); + assert_string_equal(port, "2222"); + SAFE_FREE(port); + + /* do not want port */ + rc = ssh_config_parse_uri("1:2:3::4", &username, &hostname, NULL, true); + assert_return_code(rc, errno); + assert_null(username); + assert_string_equal(hostname, "1:2:3::4"); + SAFE_FREE(hostname); +} + int torture_run_tests(void) { int rc; @@ -1922,6 +1969,8 @@ int torture_run_tests(void) setup, teardown), cmocka_unit_test_setup_teardown(torture_config_make_absolute_no_sshdir, setup_no_sshdir, teardown), + cmocka_unit_test_setup_teardown(torture_config_parse_uri, + setup, teardown), }; diff --git a/tests/unittests/torture_options.c b/tests/unittests/torture_options.c index 425d7295..8a5505a1 100644 --- a/tests/unittests/torture_options.c +++ b/tests/unittests/torture_options.c @@ -60,6 +60,20 @@ static void torture_options_set_host(void **state) { assert_non_null(session->opts.host); assert_string_equal(session->opts.host, "localhost"); + /* IPv4 address */ + rc = ssh_options_set(session, SSH_OPTIONS_HOST, "127.1.1.1"); + assert_true(rc == 0); + assert_non_null(session->opts.host); + assert_string_equal(session->opts.host, "127.1.1.1"); + assert_null(session->opts.username); + + /* IPv6 address */ + rc = ssh_options_set(session, SSH_OPTIONS_HOST, "::1"); + assert_true(rc == 0); + assert_non_null(session->opts.host); + assert_string_equal(session->opts.host, "::1"); + assert_null(session->opts.username); + rc = ssh_options_set(session, SSH_OPTIONS_HOST, "guru@meditation"); assert_true(rc == 0); assert_non_null(session->opts.host); @@ -67,12 +81,14 @@ static void torture_options_set_host(void **state) { assert_non_null(session->opts.username); assert_string_equal(session->opts.username, "guru"); + /* more @ in uri is OK -- it should go to the username */ rc = ssh_options_set(session, SSH_OPTIONS_HOST, "at@login@hostname"); assert_true(rc == 0); assert_non_null(session->opts.host); assert_string_equal(session->opts.host, "hostname"); assert_non_null(session->opts.username); assert_string_equal(session->opts.username, "at@login"); + } static void torture_options_set_ciphers(void **state) { -- 2.41.0 diff -up libssh-0.10.4/include/libssh/config_parser.h.CVE-2023-6004.patch libssh-0.10.4/include/libssh/config_parser.h --- libssh-0.10.4/include/libssh/config_parser.h.CVE-2023-6004.patch 2024-01-08 19:48:48.964006647 +0100 +++ libssh-0.10.4/include/libssh/config_parser.h 2024-01-08 19:56:38.850726165 +0100 @@ -47,13 +47,16 @@ int ssh_config_get_yesno(char **str, int * be stored or NULL if we do not care about the result. * @param[out] port Pointer to the location, where the new port will * be stored or NULL if we do not care about the result. + * @param[in] ignore_port Set to true if the we should not attempt to parse + * port number. * * @returns SSH_OK if the provided string is in format of SSH URI, * SSH_ERROR on failure */ int ssh_config_parse_uri(const char *tok, - char **username, - char **hostname, - char **port); + char **username, + char **hostname, + char **port, + bool ignore_port); #endif /* LIBSSH_CONFIG_H_ */ diff -up libssh-0.10.4/include/libssh/misc.h.CVE-2023-6004.patch libssh-0.10.4/include/libssh/misc.h --- libssh-0.10.4/include/libssh/misc.h.CVE-2023-6004.patch 2024-01-08 19:48:07.422589393 +0100 +++ libssh-0.10.4/include/libssh/misc.h 2024-01-08 19:53:33.830867829 +0100 @@ -99,4 +99,7 @@ int ssh_newline_vis(const char *string, int ssh_tmpname(char *template); char *ssh_strreplace(const char *src, const char *pattern, const char *repl); + +int ssh_check_hostname_syntax(const char *hostname); + #endif /* MISC_H_ */ diff -up libssh-0.10.4/src/misc.c.CVE-2023-6004.patch libssh-0.10.4/src/misc.c --- libssh-0.10.4/src/misc.c.CVE-2023-6004.patch 2024-01-08 19:48:26.779783823 +0100 +++ libssh-0.10.4/src/misc.c 2024-01-08 19:54:55.209685193 +0100 @@ -96,6 +96,8 @@ #define ZLIB_STRING "" #endif +#define ARPA_DOMAIN_MAX_LEN 63 + /** * @defgroup libssh_misc The SSH helper functions. * @ingroup libssh