From b07306d5bcb22550d6e9fb7f1d354a9153d5e128 Mon Sep 17 00:00:00 2001 From: TapTap Date: Mon, 21 Sep 2026 18:53:07 +0200 Subject: [PATCH] fix(config): check ssh-dest allocation and enforce MAX_FILTER_RULES client-side --- src/client/client_validation.c | 10 ++++++++++ src/shared/config.c | 27 +++++++++++++++++++++++++-- tests/test_client_cli.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/src/client/client_validation.c b/src/client/client_validation.c index 37d173f..bb129d6 100644 --- a/src/client/client_validation.c +++ b/src/client/client_validation.c @@ -102,6 +102,16 @@ bool validate_config(const Config* config) { log_message(LOG_LEVEL_ERROR, "%s", invariants_error); return false; } + /* The receiver rejects a protect-rule block with more than MAX_FILTER_RULES + entries as an opaque protocol error; reject an over-limit --filter set here, + before any network I/O, with an actionable message. send_protect_entries() + re-checks the final built count because cvs-exclude / merge rules can + expand it beyond config->filters->size. */ + if (config->filters && config->filters->size > MAX_FILTER_RULES) { + log_message(LOG_LEVEL_ERROR, "too many filter rules: %d (maximum %d)", config->filters->size, + MAX_FILTER_RULES); + return false; + } /* --protocol: FastSync has exactly one wire format, so the forced version must equal the current PROTOCOL_VERSION exactly. Rejected here, before any network I/O, rather than letting the server hit its own mismatch check. */ diff --git a/src/shared/config.c b/src/shared/config.c index 2a2941b..ba3f34d 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -230,6 +230,13 @@ Config* config_create(void) { if (!config) return NULL; config_set_defaults(config); + /* config_set_defaults() dups the default server host; a failure there leaves + * server_host NULL and would crash later consumers, so fail the whole create + * (every caller already handles a NULL return). */ + if (!config->server_host) { + free(config); + return NULL; + } return config; } @@ -686,9 +693,15 @@ int config_parse_ssh_dest(Config* config) { return daemon_dest_parse_error("invalid remote destination user@host (must not be empty or " "start with '-')", dest); - config->transport = TRANSPORT_SSH; - config->ssh_destination = str_dup(dest); + char* ssh_destination = str_dup(dest); char* path = str_dup(colon + 1); + if (!ssh_destination || !path) { + free(ssh_destination); + free(path); + return daemon_dest_parse_error("out of memory parsing remote destination", dest); + } + config->transport = TRANSPORT_SSH; + config->ssh_destination = ssh_destination; free(config->receive_root_directory); config->receive_root_directory = path; return 0; @@ -1052,6 +1065,16 @@ static bool send_protect_entries(int fd, const Config* c) { log_message(LOG_LEVEL_ERROR, "invalid filter rule: %s", err); return false; } + /* The receiver rejects any block with more than MAX_FILTER_RULES entries as a + * protocol error; refuse to emit such a frame at all. filter_base_build() + * can expand the client rule set (cvs-exclude, merge files), so this is the + * authoritative bound, not config->filters->size. */ + if (rules->count < 0 || rules->count > MAX_FILTER_RULES) { + log_message(LOG_LEVEL_ERROR, "too many filter rules: %d (maximum %d)", rules->count, + MAX_FILTER_RULES); + filter_rule_list_free(rules); + return false; + } bool ok = send_int(fd, rules->count); for (int i = 0; ok && i < rules->count; i++) { const FilterRule* r = rules->items[i]; diff --git a/tests/test_client_cli.c b/tests/test_client_cli.c index d8eb597..951ada9 100644 --- a/tests/test_client_cli.c +++ b/tests/test_client_cli.c @@ -171,6 +171,38 @@ static void test_validate_config_unified_invariants() { config_delete(cfg); } +/* The receiver enforces MAX_FILTER_RULES on the protect-rule block and would + otherwise fail the session with an opaque protocol error. The client must + accept exactly the limit and reject one more up front, before any network + I/O, with an actionable message. */ +static void test_validate_config_filter_rule_limit() { + Config* cfg = valid_client_config(); + cfg->filters = array_list_create(free); + EXPECT_NOT_NULL(cfg->filters); + for (int i = 0; i < MAX_FILTER_RULES; i++) + EXPECT_TRUE(array_list_add(cfg->filters, str_dup("- *.tmp"))); + EXPECT_TRUE(validate_config(cfg)); /* exactly the limit is accepted */ + + FILE* log_capture = tmpfile(); + EXPECT_NOT_NULL(log_capture); + log_set_file(log_capture); + EXPECT_TRUE(array_list_add(cfg->filters, str_dup("- *.bak"))); + EXPECT_FALSE(validate_config(cfg)); /* one over the limit is rejected */ + fflush(log_capture); + rewind(log_capture); + char line[512]; + bool saw_message = false; + while (fgets(line, sizeof(line), log_capture) != NULL) { + if (strstr(line, "too many filter rules") != NULL && strstr(line, "(maximum 1024)") != NULL) + saw_message = true; + } + log_set_file(NULL); + fclose(log_capture); + EXPECT_TRUE(saw_message); + + config_delete(cfg); +} + /* Test main() with --help flag (early return path, no server connection needed) */ static void test_cli_help() { /* We can't easily call main() because it calls send_files which needs a server. @@ -4824,6 +4856,7 @@ void test_client_cli() { test_validate_config_credentials_require_tls_or_loopback(); test_validate_config_delta_sendfile_constraints(); test_validate_config_unified_invariants(); + test_validate_config_filter_rule_limit(); test_cli_help(); test_cli_archive_flags(); test_cli_dry_run();