Bug: Missing NULL-checks and weak input validation in CLI argument parsing #152

New Issue

2026-07-29T18:21:43+02:00

TapTap commented

2026-07-29 18:21:43 +02:00

Description

Several CLI argument parsing paths in client_cli.c have inadequate input validation, risking crashes or silent misbehavior:

1. str_dup() results unchecked (lines 254-255)

config->send_directory = str_dup(argv[positional_args[0]]);
config->receive_root_directory = str_dup(argv[positional_args[1]]);

If malloc fails inside str_dup, the code continues with NULL pointers. The subsequent NULL check at line 271 will catch this, but the error message will be misleading ("source and destination directories are required" rather than "out of memory").

2. str_dup() for --exclude / --include unchecked (lines 120, 130)

config->exclude_patterns[config->exclude_count++] = str_dup(argv[++i]);

If str_dup returns NULL, the config stores a NULL pattern, and later calls to glob_match or strcmp may dereference it.

3. --max-size / --min-size no errno/endptr validation (lines 131-133)

config->max_size = strtoull(argv[++i], NULL, 10);

Passes NULL for endptr — garbage like --max-size abc silently becomes 0. No errno check for overflow.

4. --chunk-size no lower bound validation (line 213-215)

unsigned long long val = strtoull(argv[++i], NULL, 10);
if (val > 0)
    config->chunk_size = val;

Passes NULL for endptr. No upper bound check — chunk_size of ULLONG_MAX would cause allocation failures later.

Location

src/client/client_cli.c:120, 130-133, 213-215, 254-255

Suggested Fix

Add NULL checks after every str_dup call, with proper error message.
For --max-size, --min-size, and --chunk-size: pass an endptr and check *end != 0; also check errno for ERANGE.
Add a sensible upper bound for --chunk-size (e.g., 1 GB max).
Consider extracting a helper: static bool parse_u64(const char* str, unsigned long long* out, unsigned long long min, unsigned long long max).

Severity

High

Category

Bug / Reliability

## Description Several CLI argument parsing paths in `client_cli.c` have inadequate input validation, risking crashes or silent misbehavior: ### 1. str_dup() results unchecked (lines 254-255) ```c config->send_directory = str_dup(argv[positional_args[0]]); config->receive_root_directory = str_dup(argv[positional_args[1]]); ``` If `malloc` fails inside `str_dup`, the code continues with NULL pointers. The subsequent NULL check at line 271 will catch this, but the error message will be misleading ("source and destination directories are required" rather than "out of memory"). ### 2. str_dup() for --exclude / --include unchecked (lines 120, 130) ```c config->exclude_patterns[config->exclude_count++] = str_dup(argv[++i]); ``` If `str_dup` returns NULL, the config stores a NULL pattern, and later calls to `glob_match` or `strcmp` may dereference it. ### 3. --max-size / --min-size no errno/endptr validation (lines 131-133) ```c config->max_size = strtoull(argv[++i], NULL, 10); ``` Passes `NULL` for `endptr` — garbage like `--max-size abc` silently becomes 0. No `errno` check for overflow. ### 4. --chunk-size no lower bound validation (line 213-215) ```c unsigned long long val = strtoull(argv[++i], NULL, 10); if (val > 0) config->chunk_size = val; ``` Passes `NULL` for `endptr`. No upper bound check — chunk_size of ULLONG_MAX would cause allocation failures later. ## Location `src/client/client_cli.c:120, 130-133, 213-215, 254-255` ## Suggested Fix 1. Add NULL checks after every `str_dup` call, with proper error message. 2. For `--max-size`, `--min-size`, and `--chunk-size`: pass an `endptr` and check `*end != 0`; also check `errno` for `ERANGE`. 3. Add a sensible upper bound for `--chunk-size` (e.g., 1 GB max). 4. Consider extracting a helper: `static bool parse_u64(const char* str, unsigned long long* out, unsigned long long min, unsigned long long max)`. ## Severity High ## Category Bug / Reliability

TapTap referenced this issue from a commit

2026-07-29 18:29:22 +02:00

fix: add NULL-checks and input validation in CLI argument parsing (#152)

TapTap referenced this issue from a commit

2026-07-29 18:29:33 +02:00

fix: add NULL-checks and input validation in CLI argument parsing (#152)

TapTap referenced a pull request that will close this issue

2026-07-29 18:29:38 +02:00

fix: add NULL-checks and input validation in CLI argument parsing (#152) #153

TapTap referenced this issue

2026-07-29 18:36:42 +02:00

Batch fix: NULL checks, input validation, and code refactoring (#152, #149, #150, #151) #171