From c75ccfd80b6904713cb7e03a4a55453f877e7850 Mon Sep 17 00:00:00 2001 From: TapTap Date: Wed, 29 Jul 2026 18:29:21 +0200 Subject: [PATCH] fix: add NULL-checks and input validation in CLI argument parsing (#152) - Add NULL-checks for all str_dup() calls in argument parsing - Replace atoi with strtol + endptr validation for port numbers - Add errno/endptr validation for strtoull calls (--max-size, --min-size) - Check str_dup result for exclude/include patterns, server-host, backup-dir - Validate positional directory arguments for allocation failure --- src/client/client_cli.c | 108 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 99 insertions(+), 9 deletions(-) diff --git a/src/client/client_cli.c b/src/client/client_cli.c index ba17a21..5fd388c 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -99,7 +99,13 @@ static int read_patterns_from_file(const char* filepath, char*** patterns, int* return -1; } *patterns = tmp; - (*patterns)[(*count)++] = str_dup(p); + (*patterns)[*count] = str_dup(p); + if (!(*patterns)[*count]) { + fprintf(stderr, "Error: memory allocation failed for pattern\n"); + fclose(fp); + return -1; + } + (*count)++; } fclose(fp); return 0; @@ -142,7 +148,15 @@ int main(int argc, char* argv[]) { } else if (strcmp(argv[i], "-n") == 0 || strcmp(argv[i], "--dry-run") == 0) { config->dry_run = true; } else if (strcmp(argv[i], "-p") == 0 && i + 1 < argc) { - config->ssh_port = atoi(argv[++i]); + char* end; + errno = 0; + long val = strtol(argv[++i], &end, 10); + if (errno != 0 || *end != '\0' || val <= 0 || val > 65535) { + fprintf(stderr, "Error: -p must be a valid port number (1-65535)\n"); + exit_code = 1; + goto cleanup; + } + config->ssh_port = (int)val; } else if (strcmp(argv[i], "--delete") == 0) { config->use_delete = true; } else if (strcmp(argv[i], "--exclude") == 0 && i + 1 < argc) { @@ -153,7 +167,13 @@ int main(int argc, char* argv[]) { goto cleanup; } config->exclude_patterns = tmp; - config->exclude_patterns[config->exclude_count++] = str_dup(argv[++i]); + config->exclude_patterns[config->exclude_count] = str_dup(argv[++i]); + if (!config->exclude_patterns[config->exclude_count]) { + fprintf(stderr, "Error: memory allocation failed for exclude pattern\n"); + exit_code = 1; + goto cleanup; + } + config->exclude_count++; } else if (strcmp(argv[i], "--include") == 0 && i + 1 < argc) { char** tmp = realloc(config->include_patterns, (config->include_count + 1) * sizeof(char*)); if (!tmp) { @@ -162,11 +182,31 @@ int main(int argc, char* argv[]) { goto cleanup; } config->include_patterns = tmp; - config->include_patterns[config->include_count++] = str_dup(argv[++i]); + config->include_patterns[config->include_count] = str_dup(argv[++i]); + if (!config->include_patterns[config->include_count]) { + fprintf(stderr, "Error: memory allocation failed for include pattern\n"); + exit_code = 1; + goto cleanup; + } + config->include_count++; } else if (strcmp(argv[i], "--max-size") == 0 && i + 1 < argc) { - config->max_size = strtoull(argv[++i], NULL, 10); + char* end; + errno = 0; + config->max_size = strtoull(argv[++i], &end, 10); + if (errno != 0 || *end != '\0') { + fprintf(stderr, "Error: --max-size must be a valid non-negative integer\n"); + exit_code = 1; + goto cleanup; + } } else if (strcmp(argv[i], "--min-size") == 0 && i + 1 < argc) { - config->min_size = strtoull(argv[++i], NULL, 10); + char* end; + errno = 0; + config->min_size = strtoull(argv[++i], &end, 10); + if (errno != 0 || *end != '\0') { + fprintf(stderr, "Error: --min-size must be a valid non-negative integer\n"); + exit_code = 1; + goto cleanup; + } } else if (strcmp(argv[i], "--incremental") == 0) { config->use_incremental = true; } else if (strcmp(argv[i], "--delta") == 0) { @@ -198,9 +238,19 @@ int main(int argc, char* argv[]) { } else if (strcmp(argv[i], "--source-dir") == 0 && i + 1 < argc) { free(config->send_directory); config->send_directory = str_dup(argv[++i]); + if (!config->send_directory) { + fprintf(stderr, "Error: memory allocation failed\n"); + exit_code = 1; + goto cleanup; + } } else if (strcmp(argv[i], "--dest-dir") == 0 && i + 1 < argc) { free(config->receive_root_directory); config->receive_root_directory = str_dup(argv[++i]); + if (!config->receive_root_directory) { + fprintf(stderr, "Error: memory allocation failed\n"); + exit_code = 1; + goto cleanup; + } } else if (strcmp(argv[i], "--save-to-disk") == 0) { config->save_to_disk = true; } else if (strcmp(argv[i], "-M") == 0 || strcmp(argv[i], "--preserve") == 0) { @@ -218,8 +268,21 @@ int main(int argc, char* argv[]) { } else if (strcmp(argv[i], "--server-host") == 0 && i + 1 < argc) { free(config->server_host); config->server_host = str_dup(argv[++i]); + if (!config->server_host) { + fprintf(stderr, "Error: memory allocation failed\n"); + exit_code = 1; + goto cleanup; + } } else if (strcmp(argv[i], "--server-port") == 0 && i + 1 < argc) { - config->server_port = atoi(argv[++i]); + char* end; + errno = 0; + long val = strtol(argv[++i], &end, 10); + if (errno != 0 || *end != '\0' || val <= 0 || val > 65535) { + fprintf(stderr, "Error: --server-port must be a valid port number (1-65535)\n"); + exit_code = 1; + goto cleanup; + } + config->server_port = (int)val; } else if (strcmp(argv[i], "--bwlimit") == 0 && i + 1 < argc) { char* end; errno = 0; @@ -274,6 +337,11 @@ int main(int argc, char* argv[]) { config->backup = true; } else if (strcmp(argv[i], "--backup-dir") == 0 && i + 1 < argc) { config->backup_dir = str_dup(argv[++i]); + if (!config->backup_dir) { + fprintf(stderr, "Error: memory allocation failed\n"); + exit_code = 1; + goto cleanup; + } } else if (strcmp(argv[i], "--stats") == 0) { config->stats = true; } else if (strcmp(argv[i], "--max-depth") == 0 && i + 1 < argc) { @@ -316,6 +384,11 @@ int main(int argc, char* argv[]) { } else if (strcmp(argv[i], "--fastsync-server-path") == 0 && i + 1 < argc) { free(config->fastsync_server_path); config->fastsync_server_path = str_dup(argv[++i]); + if (!config->fastsync_server_path) { + fprintf(stderr, "Error: memory allocation failed\n"); + exit_code = 1; + goto cleanup; + } } else if (strcmp(argv[i], "-v") == 0 || strcmp(argv[i], "--verbose") == 0) { set_log_level(LOG_LEVEL_DEBUG); } else if (argv[i][0] == '-') { @@ -340,6 +413,11 @@ int main(int argc, char* argv[]) { free(config->receive_root_directory); config->send_directory = str_dup(argv[positional_args[0]]); config->receive_root_directory = str_dup(argv[positional_args[1]]); + if (!config->send_directory || !config->receive_root_directory) { + fprintf(stderr, "Error: memory allocation failed for directory paths\n"); + exit_code = 1; + goto cleanup; + } config->save_to_disk = true; config_parse_ssh_dest(config); @@ -349,10 +427,22 @@ int main(int argc, char* argv[]) { exit_code = 1; goto cleanup; } else { - if (!config->send_directory && env_source) + if (!config->send_directory && env_source) { config->send_directory = str_dup((char*)env_source); - if (!config->receive_root_directory && env_dest) + if (!config->send_directory) { + fprintf(stderr, "Error: memory allocation failed for source directory\n"); + exit_code = 1; + goto cleanup; + } + } + if (!config->receive_root_directory && env_dest) { config->receive_root_directory = str_dup((char*)env_dest); + if (!config->receive_root_directory) { + fprintf(stderr, "Error: memory allocation failed for destination directory\n"); + exit_code = 1; + goto cleanup; + } + } } if (!config->send_directory || !config->receive_root_directory) { -- 2.52.0