diff --git a/CMakeLists.txt b/CMakeLists.txt index 09c10f6..a55d93e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -26,9 +26,9 @@ elseif(NOT SANITIZER STREQUAL "none") endif() # --- Strict warnings option --- -option(STRICT_WARNINGS "Enable strict warnings (Wextra, Wpedantic, Werror)" OFF) +option(STRICT_WARNINGS "Enable strict warnings (Wextra, Wpedantic, Wformat-signedness, Werror)" OFF) if(STRICT_WARNINGS) - add_compile_options(-Wextra -Wpedantic -Werror) + add_compile_options(-Wextra -Wpedantic -Wformat-signedness -Werror) endif() # --- Coverage option --- diff --git a/src/client/client_cli.c b/src/client/client_cli.c index 8565b63..fb0411b 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -3296,7 +3296,7 @@ int main(int argc, char* argv[]) { exit_code = 1; } } else if (config->use_multithreading) { - exit_code = send_files_multithreaded(&config); + exit_code = send_files_multithreaded(config); } else { exit_code = send_files(config); } diff --git a/src/client/client_send.c b/src/client/client_send.c index c777d52..e5c71f4 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -37,8 +37,6 @@ #include #include -#define STREAM_THRESHOLD (64ULL * 1024 * 1024) - /* Aggregate loaded payload bytes the sender may buffer across the loader queue and the chunk in flight. Sending one chunk adds up to ~2 * MAX_CHUNK_SIZE of transient serialize/compress buffers on top of the queued payloads, so this @@ -1115,7 +1113,7 @@ static Client* connect_transfer_client(const Config* config) { return NULL; } return client_connect_ssh(config->ssh_destination, config->ssh_port, - config->fastsync_server_path, config->old_args, config->rsh_command, + config->fastsync_server_path, config->rsh_command, config->blocking_io, config->remote_options, config->remote_option_count); } @@ -3617,10 +3615,9 @@ send_fail: return ret; } -int send_files_multithreaded(Config** config_ptr) { - if (!config_ptr || !*config_ptr) +int send_files_multithreaded(Config* config) { + if (!config) return 1; - Config* config = *config_ptr; if (config->list_only) return send_list_only(config); if (config->dry_run) diff --git a/src/client/client_send.h b/src/client/client_send.h index f8f3706..c8146c9 100644 --- a/src/client/client_send.h +++ b/src/client/client_send.h @@ -22,7 +22,7 @@ void client_set_abort_armed(bool armed); * never free it, and the caller retains ownership (freeing it with * config_delete() once the call returns). */ int send_files(Config* config); -int send_files_multithreaded(Config** config); +int send_files_multithreaded(Config* config); /* rsync's --ignore-errors deletion gate: with no I/O error during the scan the * deletion phase always proceeds; with one it is suppressed unless * `--ignore-errors` was given. Exposed so the decision can be unit-tested diff --git a/src/shared/chunk.c b/src/shared/chunk.c index ab0fb4c..54e8730 100644 --- a/src/shared/chunk.c +++ b/src/shared/chunk.c @@ -17,8 +17,9 @@ #include "protocol.h" #include "utils.h" -/* Maximum individual file data size within a chunk (64 MB) */ -#define MAX_FILE_DATA_SIZE (64ULL * 1024 * 1024) +/* Maximum individual file data size within a chunk (64 MB). Distinct from the + * receiver's whole-file MAX_FILE_DATA_SIZE (256 MB) in file_receive.c. */ +#define MAX_CHUNK_FILE_DATA_SIZE (64ULL * 1024 * 1024) #define MAX_FILES_PER_CHUNK 65536U /* Reserve `charge` against `session`'s connection budget. This mirrors the @@ -382,9 +383,9 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { } // Reject individual file data larger than the maximum allowed size. - if (file_data_size > MAX_FILE_DATA_SIZE) { + if (file_data_size > MAX_CHUNK_FILE_DATA_SIZE) { log_message(LOG_LEVEL_ERROR, "File data size %zu exceeds maximum %llu", file_data_size, - (unsigned long long)MAX_FILE_DATA_SIZE); + (unsigned long long)MAX_CHUNK_FILE_DATA_SIZE); goto error; } diff --git a/src/shared/file.c b/src/shared/file.c index 9ec981e..bcb46f4 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -26,11 +26,6 @@ #include "protocol.h" #include "xattr.h" -/* Files larger than this are not loaded whole for transfer (the sender streams - * them); a whole-file digest is computed from the path instead. Kept in sync - * with the sender's streaming threshold. */ -#define STREAM_THRESHOLD (64ULL * 1024 * 1024) - static bool write_all(int fd, const void* data, unsigned long long size) { const unsigned char* p = data; unsigned long long done = 0; diff --git a/src/shared/protocol.h b/src/shared/protocol.h index 73b9132..d13c3f7 100644 --- a/src/shared/protocol.h +++ b/src/shared/protocol.h @@ -28,6 +28,10 @@ /* Maximum chunk size (64 MB) — prevents unbounded allocation from the wire */ #define MAX_CHUNK_SIZE (64ULL * 1024 * 1024) +/* Files larger than this are not kept fully in memory while loading: the + * loader skips them so the sender streams from the path, and file_checksum + * hashes them from disk in bounded buffers instead of forcing a full load. */ +#define STREAM_THRESHOLD (64ULL * 1024 * 1024) #define MAX_MANIFEST_ENTRIES (1024 * 1024) /* Aggregate bytes retained by one received deletion manifest. */ #define MAX_MANIFEST_BYTES (16ULL * 1024 * 1024) diff --git a/src/shared/transport_ssh.c b/src/shared/transport_ssh.c index 7236133..1be7f21 100644 --- a/src/shared/transport_ssh.c +++ b/src/shared/transport_ssh.c @@ -87,7 +87,7 @@ static int parse_remote_dest(const char* dest, RemoteDest* r) { return 0; } -char* ssh_build_remote_command(const char* server_path, bool old_args, char* const* remote_options, +char* ssh_build_remote_command(const char* server_path, char* const* remote_options, int remote_option_count) { const char* path = server_path ? server_path : "fastsync-server"; const char* suffix = " --stdio"; @@ -105,9 +105,7 @@ char* ssh_build_remote_command(const char* server_path, bool old_args, char* con shell word (remote options below reuse the same escaping), then " --stdio". Quoting the path is the only injection-safe construction: an unquoted path would carry shell metacharacters straight into the remote - shell command. --old-args is kept for CLI/ABI compatibility but no longer - disables that protection. */ - (void)old_args; + shell command. (rsync's --old-args no longer disables that protection.) */ size_t quote_count = 0; for (const char* p = path; *p; p++) if (*p == '\'') @@ -296,8 +294,8 @@ void ssh_free_client_argv(char** argv) { } Client* client_connect_ssh(const char* destination, int port, const char* server_path, - bool old_args, const char* rsh_command, bool blocking_io, - char* const* remote_options, int remote_option_count) { + const char* rsh_command, bool blocking_io, char* const* remote_options, + int remote_option_count) { RemoteDest r; if (parse_remote_dest(destination, &r) != 0) { char* escaped = output_escape(destination, false); @@ -376,7 +374,7 @@ Client* client_connect_ssh(const char* destination, int port, const char* server snprintf(ssh_user, ssh_user_len, "%s", r.host); char* remote_command = - ssh_build_remote_command(server_path, old_args, remote_options, remote_option_count); + ssh_build_remote_command(server_path, remote_options, remote_option_count); if (!remote_command) ssh_child_setup_failed(exec_pipe[1]); char** ssh_argv = ssh_build_client_argv(rsh_command, port, ssh_user, remote_command); diff --git a/src/shared/transport_ssh.h b/src/shared/transport_ssh.h index 08908ce..0ed923e 100644 --- a/src/shared/transport_ssh.h +++ b/src/shared/transport_ssh.h @@ -4,17 +4,17 @@ #include "transport_tcp.h" Client* client_connect_ssh(const char* destination, int port, const char* server_path, - bool old_args, const char* rsh_command, bool blocking_io, - char* const* remote_options, int remote_option_count); + const char* rsh_command, bool blocking_io, char* const* remote_options, + int remote_option_count); /* Build the escaped remote-shell command string (the server program path always * quoted as one remote-shell word, followed by ` --stdio` and each * --remote-option value appended as an individually single-quoted shell word). - * `old_args` is accepted for CLI/ABI compatibility but no longer disables - * quoting: the path is always escaped so a metacharacter-bearing - * --rsync-path can never be interpreted by the remote shell. Every - * --remote-option value is individually escaped with the '\'' sequence and - * values with empty/control characters are rejected at the CLI parse layer. */ -char* ssh_build_remote_command(const char* server_path, bool old_args, char* const* remote_options, + * The path is always escaped so a metacharacter-bearing --rsync-path can never + * be interpreted by the remote shell (the --old-args no-op does not disable + * quoting). Every --remote-option value is individually escaped with the '\'' + * sequence and values with empty/control characters are rejected at the CLI + * parse layer. */ +char* ssh_build_remote_command(const char* server_path, char* const* remote_options, int remote_option_count); /* Build the NULL-terminated child argv for the remote-shell client (argv[0] is * the exec/execvp program). rsh_command is whitespace-split into leading argv diff --git a/tests/test_transport_ssh.c b/tests/test_transport_ssh.c index 2889444..a99630f 100644 --- a/tests/test_transport_ssh.c +++ b/tests/test_transport_ssh.c @@ -5,13 +5,13 @@ static void test_ssh_connect_invalid_dest_no_colon() { /* cppcheck-suppress constVariablePointer */ Client* client = - client_connect_ssh("invalid-destination-no-colon", 22, NULL, false, NULL, false, NULL, 0); + client_connect_ssh("invalid-destination-no-colon", 22, NULL, NULL, false, NULL, 0); EXPECT_NULL(client); } static void test_ssh_connect_invalid_dest_empty() { /* cppcheck-suppress constVariablePointer */ - Client* client = client_connect_ssh("", 22, NULL, false, NULL, false, NULL, 0); + Client* client = client_connect_ssh("", 22, NULL, NULL, false, NULL, 0); EXPECT_NULL(client); } @@ -22,7 +22,7 @@ static void test_ssh_connect_malformed() { setenv("PATH", "", 1); /* cppcheck-suppress constVariablePointer */ - Client* client = client_connect_ssh(":", 22, NULL, false, NULL, false, NULL, 0); + Client* client = client_connect_ssh(":", 22, NULL, NULL, false, NULL, 0); if (saved_path) { setenv("PATH", saved_path, 1); @@ -38,7 +38,7 @@ static void test_ssh_connect_malformed() { * The function launches ssh which will fail to connect, returns a Client. */ static void test_ssh_connect_unreachable() { Client* client = - client_connect_ssh("nonexistent.invalid:/remote/path", 22, NULL, false, NULL, false, NULL, 0); + client_connect_ssh("nonexistent.invalid:/remote/path", 22, NULL, NULL, false, NULL, 0); if (client != NULL) { client_disconnect(client); client_delete(client); @@ -47,23 +47,13 @@ static void test_ssh_connect_unreachable() { } static void test_ssh_remote_command_argument_modes() { - char* command = ssh_build_remote_command("fast sync; touch /tmp/pwned", false, NULL, 0); + char* command = ssh_build_remote_command("fast sync; touch /tmp/pwned", NULL, 0); EXPECT_EQ_STR(command, "'fast sync; touch /tmp/pwned' --stdio"); free(command); - command = ssh_build_remote_command("fast'sync", false, NULL, 0); + command = ssh_build_remote_command("fast'sync", NULL, 0); EXPECT_EQ_STR(command, "'fast'\\''sync' --stdio"); free(command); - - /* --old-args no longer disables injection-safe quoting: the path is still one - single-quoted word, even when it carries shell metacharacters. */ - command = ssh_build_remote_command("fast sync; touch /tmp/pwned", true, NULL, 0); - EXPECT_EQ_STR(command, "'fast sync; touch /tmp/pwned' --stdio"); - free(command); - - command = ssh_build_remote_command("fast'sync; rm -rf /", true, NULL, 0); - EXPECT_EQ_STR(command, "'fast'\\''sync; rm -rf /' --stdio"); - free(command); } /* The build for a single-word argv is [prog, six -o args, "--", user, command]. */ @@ -120,12 +110,12 @@ static void test_ssh_build_client_argv_whitespace_command_and_port() { * returned and no command can run. */ static void test_ssh_connect_rejects_option_host() { /* cppcheck-suppress constVariablePointer */ - Client* client = client_connect_ssh("-oProxyCommand=touch /tmp/pwned:/remote", 22, NULL, false, - NULL, false, NULL, 0); + Client* client = + client_connect_ssh("-oProxyCommand=touch /tmp/pwned:/remote", 22, NULL, NULL, false, NULL, 0); EXPECT_NULL(client); - client = client_connect_ssh("-evil:/remote", 22, NULL, false, NULL, false, NULL, 0); + client = client_connect_ssh("-evil:/remote", 22, NULL, NULL, false, NULL, 0); EXPECT_NULL(client); - client = client_connect_ssh("user@:/remote", 22, NULL, false, NULL, false, NULL, 0); + client = client_connect_ssh("user@:/remote", 22, NULL, NULL, false, NULL, 0); EXPECT_NULL(client); } @@ -135,13 +125,13 @@ static void test_ssh_connect_rejects_option_host() { * ssh_build_remote_command safety boundary for the server path. */ static void test_ssh_remote_command_with_remote_options() { char* noop[] = {"--allow-delete"}; - char* command = ssh_build_remote_command("fastsync-server", false, noop, 1); + char* command = ssh_build_remote_command("fastsync-server", noop, 1); EXPECT_EQ_STR(command, "'fastsync-server' --stdio '--allow-delete'"); free(command); /* Multiple options append in order, each as its own quoted word. */ char* multi[] = {"-v", "--allow-delete"}; - command = ssh_build_remote_command("srv", false, multi, 2); + command = ssh_build_remote_command("srv", multi, 2); EXPECT_EQ_STR(command, "'srv' --stdio '-v' '--allow-delete'"); free(command); @@ -150,29 +140,24 @@ static void test_ssh_remote_command_with_remote_options() { break out into an arbitrary remote command. */ char* val = strdup("--x=un'der; touch /tmp/pwned"); char* dangerous[1] = {val}; - command = ssh_build_remote_command("srv", false, dangerous, 1); + command = ssh_build_remote_command("srv", dangerous, 1); EXPECT_EQ_STR(command, "'srv' --stdio '--x=un'\\''der; touch /tmp/pwned'"); free(command); free(val); - - /* --old-args still quotes both the server path and the remote options. */ - command = ssh_build_remote_command("srv", true, multi, 2); - EXPECT_EQ_STR(command, "'srv' --stdio '-v' '--allow-delete'"); - free(command); } /* The remote command builder refuses to forward an empty or control-character * remote option (defense-in-depth independent of the CLI validation). */ static void test_ssh_remote_command_rejects_bad_options() { char* empty[] = {""}; - EXPECT_NULL(ssh_build_remote_command("srv", false, empty, 1)); + EXPECT_NULL(ssh_build_remote_command("srv", empty, 1)); char nl = '\n'; char* newline[] = {&nl}; - EXPECT_NULL(ssh_build_remote_command("srv", false, newline, 1)); + EXPECT_NULL(ssh_build_remote_command("srv", newline, 1)); char* with_null[] = {NULL}; - EXPECT_NULL(ssh_build_remote_command("srv", false, with_null, 1)); + EXPECT_NULL(ssh_build_remote_command("srv", with_null, 1)); } void test_transport_ssh() {