From ec6160a8b9029e2ab23d73bf59f0a52242914038 Mon Sep 17 00:00:00 2001 From: TapTap Date: Thu, 3 Sep 2026 21:34:32 +0200 Subject: [PATCH] fix: propagate SSH child setup failures --- src/shared/transport_ssh.c | 27 ++++++++++++++++----------- tests/test_transport_ssh.c | 23 ++++++++++++++--------- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/shared/transport_ssh.c b/src/shared/transport_ssh.c index 8261191..2a924c9 100644 --- a/src/shared/transport_ssh.c +++ b/src/shared/transport_ssh.c @@ -16,6 +16,12 @@ typedef struct { char* remote_path; } RemoteDest; +static void ssh_child_setup_failed(int status_fd) { + ssize_t wret = write(status_fd, "x", 1); + (void)wret; + _exit(1); +} + static void remote_dest_destroy(RemoteDest* r) { free(r->user); free(r->host); @@ -157,11 +163,12 @@ Client* client_connect_ssh(const char* destination, int port, const char* server if (pid == 0) { close(sv[0]); close(exec_pipe[0]); - fcntl(exec_pipe[1], F_SETFD, FD_CLOEXEC); - if (sv[1] != STDIN_FILENO) - dup2(sv[1], STDIN_FILENO); - if (sv[1] != STDOUT_FILENO) - dup2(sv[1], STDOUT_FILENO); + if (fcntl(exec_pipe[1], F_SETFD, FD_CLOEXEC) < 0) + ssh_child_setup_failed(exec_pipe[1]); + if (sv[1] != STDIN_FILENO && dup2(sv[1], STDIN_FILENO) < 0) + ssh_child_setup_failed(exec_pipe[1]); + if (sv[1] != STDOUT_FILENO && dup2(sv[1], STDOUT_FILENO) < 0) + ssh_child_setup_failed(exec_pipe[1]); if (sv[1] > 1) close(sv[1]); @@ -172,7 +179,7 @@ Client* client_connect_ssh(const char* destination, int port, const char* server ssh_user_len = strlen(r.host) + 1; char* ssh_user = malloc(ssh_user_len); if (!ssh_user) - _exit(1); + ssh_child_setup_failed(exec_pipe[1]); if (r.user && r.user[0] != '\0') snprintf(ssh_user, ssh_user_len, "%s@%s", r.user, r.host); else @@ -183,7 +190,7 @@ Client* client_connect_ssh(const char* destination, int port, const char* server char port_str[16]; char* remote_command = ssh_build_remote_command(server_path, old_args); if (!remote_command) - _exit(1); + ssh_child_setup_failed(exec_pipe[1]); ssh_argv[ac++] = "ssh"; ssh_argv[ac++] = "-o"; ssh_argv[ac++] = "Compression=no"; @@ -201,9 +208,7 @@ Client* client_connect_ssh(const char* destination, int port, const char* server ssh_argv[ac] = NULL; execvp("ssh", ssh_argv); log_perror("exec of ssh failed"); - ssize_t wret = write(exec_pipe[1], "x", 1); - (void)wret; - _exit(1); + ssh_child_setup_failed(exec_pipe[1]); } close(sv[1]); @@ -213,7 +218,7 @@ Client* client_connect_ssh(const char* destination, int port, const char* server ssize_t n = read(exec_pipe[0], &exec_status, 1); close(exec_pipe[0]); - if (n > 0) { + if (n != 0) { close(sv[0]); waitpid(pid, NULL, 0); remote_dest_destroy(&r); diff --git a/tests/test_transport_ssh.c b/tests/test_transport_ssh.c index bb83cd9..7a2f3d6 100644 --- a/tests/test_transport_ssh.c +++ b/tests/test_transport_ssh.c @@ -14,18 +14,23 @@ static void test_ssh_connect_invalid_dest_empty() { EXPECT_NULL(client); } -/* Test client_connect_ssh with malformed destination (just a colon). - * parse_remote_dest succeeds, ssh is exec'd and fails, but the function - * creates a Client that must be cleaned up. */ +/* A child that cannot exec ssh must not be returned as a successful client. */ static void test_ssh_connect_malformed() { + const char* old_path = getenv("PATH"); + char* saved_path = old_path ? strdup(old_path) : NULL; + setenv("PATH", "", 1); + + /* cppcheck-suppress constVariablePointer */ Client* client = client_connect_ssh(":", 22, NULL, false); - /* ssh binary exists, so exec succeeds; the function returns a Client. - * We just verify it doesn't crash and clean up properly. */ - if (client != NULL) { - client_disconnect(client); - client_delete(client); + + if (saved_path) { + setenv("PATH", saved_path, 1); + free(saved_path); + } else { + unsetenv("PATH"); } - EXPECT_TRUE(true); + + EXPECT_NULL(client); } /* Test client_connect_ssh with valid format but unreachable host.