From 1ab8a35c5db711386abc69ed29d3a0a7bc81ee21 Mon Sep 17 00:00:00 2001 From: Rui Li Date: Fri, 6 Mar 2026 21:31:44 -0800 Subject: [PATCH] Add strict validation mode to ssh_config_parse_uri in config_parser Signed-off-by: Rui Li Reviewed-by: Jakub Jelen --- include/libssh/config_parser.h | 8 ++++++- src/config.c | 5 +++-- src/config_parser.c | 31 +++++++++++++++++++++----- src/options.c | 2 +- tests/unittests/torture_config.c | 38 +++++++++++++++++++++++++++----- 5 files changed, 68 insertions(+), 16 deletions(-) diff --git a/include/libssh/config_parser.h b/include/libssh/config_parser.h index f5d1fee8..434943f7 100644 --- a/include/libssh/config_parser.h +++ b/include/libssh/config_parser.h @@ -54,6 +54,11 @@ int ssh_config_get_yesno(char **str, int notfound); * be stored or NULL if we do not care about the result. * @param[in] ignore_port Set to true if we should not attempt to parse * port number. + * @param[in] strict Set to true to validate hostname against RFC1035 + * (for resolving to a real host). + * Set to false to only reject shell metacharacters + * (allowing config aliases with non-RFC1035 chars + * like underscores, resolved later via Hostname). * * @returns SSH_OK if the provided string is in format of SSH URI, * SSH_ERROR on failure @@ -62,7 +67,8 @@ int ssh_config_parse_uri(const char *tok, char **username, char **hostname, char **port, - bool ignore_port); + bool ignore_port, + bool strict); /** * @brief: Parse the ProxyJump configuration line and if parsing, diff --git a/src/config.c b/src/config.c index 12eb3a71..2f4ea63a 100644 --- a/src/config.c +++ b/src/config.c @@ -544,6 +544,7 @@ ssh_config_parse_proxy_jump(ssh_session session, const char *s, bool do_parsing) &jump_host->username, &jump_host->hostname, &port, + false, false); if (rv != SSH_OK) { ssh_set_error_invalid(session); @@ -566,7 +567,7 @@ ssh_config_parse_proxy_jump(ssh_session session, const char *s, bool do_parsing) } } else if (parse_entry) { /* We actually care only about the first item */ - rv = ssh_config_parse_uri(cp, &username, &hostname, &port, false); + rv = ssh_config_parse_uri(cp, &username, &hostname, &port, false, false); if (rv != SSH_OK) { ssh_set_error_invalid(session); goto out; @@ -582,7 +583,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, false); + rv = ssh_config_parse_uri(cp, NULL, NULL, NULL, false, false); if (rv != SSH_OK) { ssh_set_error_invalid(session); goto out; diff --git a/src/config_parser.c b/src/config_parser.c index 06264f84..8ee2153a 100644 --- a/src/config_parser.c +++ b/src/config_parser.c @@ -172,7 +172,8 @@ int ssh_config_parse_uri(const char *tok, char **username, char **hostname, char **port, - bool ignore_port) + bool ignore_port, + bool strict) { char *endp = NULL; long port_n; @@ -243,13 +244,31 @@ 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) { + if (strict) { + /* 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; + } + } + } else { + /* Reject shell metacharacters to allow config aliases with + * non-RFC1035 chars (e.g. %, _). Modeled on OpenSSH's + * valid_hostname() in ssh.c. */ + const char *c = NULL; + if ((*hostname)[0] == '-') { goto error; } + for (c = *hostname; *c != '\0'; c++) { + char *is_meta = strchr("'`\"$\\;&<>|(){},", *c); + int is_space = isspace((unsigned char)*c); + int is_ctrl = iscntrl((unsigned char)*c); + if (is_meta != NULL || is_space || is_ctrl) { + goto error; + } + } } } /* Skip also the closing bracket */ diff --git a/src/options.c b/src/options.c index 7c4a1055..d74cc69a 100644 --- a/src/options.c +++ b/src/options.c @@ -718,7 +718,7 @@ int ssh_options_set(ssh_session session, enum ssh_options_e type, return -1; } else { char *username = NULL, *hostname = NULL; - rc = ssh_config_parse_uri(value, &username, &hostname, NULL, true); + rc = ssh_config_parse_uri(value, &username, &hostname, NULL, true, true); if (rc != SSH_OK) { ssh_set_error_invalid(session); return -1; diff --git a/tests/unittests/torture_config.c b/tests/unittests/torture_config.c index 7ac9d9dc..89687872 100644 --- a/tests/unittests/torture_config.c +++ b/tests/unittests/torture_config.c @@ -2762,21 +2762,21 @@ static void torture_config_parse_uri(void **state) (void)state; /* unused */ - rc = ssh_config_parse_uri("localhost", &username, &hostname, &port, false); + rc = ssh_config_parse_uri("localhost", &username, &hostname, &port, false, true); 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); + rc = ssh_config_parse_uri("1.2.3.4", &username, &hostname, &port, false, true); 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); + rc = ssh_config_parse_uri("1.2.3.4:2222", &username, &hostname, &port, false, true); assert_return_code(rc, errno); assert_null(username); assert_string_equal(hostname, "1.2.3.4"); @@ -2784,7 +2784,7 @@ static void torture_config_parse_uri(void **state) assert_string_equal(port, "2222"); SAFE_FREE(port); - rc = ssh_config_parse_uri("[1:2:3::4]:2222", &username, &hostname, &port, false); + rc = ssh_config_parse_uri("[1:2:3::4]:2222", &username, &hostname, &port, false, true); assert_return_code(rc, errno); assert_null(username); assert_string_equal(hostname, "1:2:3::4"); @@ -2793,13 +2793,39 @@ static void torture_config_parse_uri(void **state) SAFE_FREE(port); /* do not want port */ - rc = ssh_config_parse_uri("1:2:3::4", &username, &hostname, NULL, true); + rc = ssh_config_parse_uri("1:2:3::4", &username, &hostname, NULL, true, true); assert_return_code(rc, errno); assert_null(username); assert_string_equal(hostname, "1:2:3::4"); SAFE_FREE(hostname); - rc = ssh_config_parse_uri("user -name@", &username, NULL, NULL, true); + rc = ssh_config_parse_uri("user -name@", &username, NULL, NULL, true, true); + assert_int_equal(rc, SSH_ERROR); + + /* Non-strict accepts non-RFC1035 chars (e.g. _, %) */ + rc = ssh_config_parse_uri("customer_1", &username, &hostname, NULL, true, false); + assert_return_code(rc, errno); + assert_null(username); + assert_string_equal(hostname, "customer_1"); + SAFE_FREE(hostname); + + rc = ssh_config_parse_uri("admin@%prod", &username, &hostname, NULL, true, false); + assert_return_code(rc, errno); + assert_string_equal(username, "admin"); + assert_string_equal(hostname, "%prod"); + SAFE_FREE(username); + SAFE_FREE(hostname); + + /* Strict rejects what non-strict accepts */ + rc = ssh_config_parse_uri("customer_1", &username, &hostname, NULL, true, true); + assert_int_equal(rc, SSH_ERROR); + + /* Non-strict rejects shell metacharacters */ + rc = ssh_config_parse_uri("host;cmd", &username, &hostname, NULL, true, false); + assert_int_equal(rc, SSH_ERROR); + + /* Non-strict rejects leading dash */ + rc = ssh_config_parse_uri("-host", &username, &hostname, NULL, true, false); assert_int_equal(rc, SSH_ERROR); }