diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index ffb72df..f21867c 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -1696,9 +1696,9 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { return NULL; } - if (has_path_traversal(check_path)) { + if (check_path[0] == '\0' || has_path_traversal(check_path)) { char* escaped_path = output_escape(check_path, log_get_8_bit_output()); - log_message(LOG_LEVEL_ERROR, "Path traversal detected: %s", + log_message(LOG_LEVEL_ERROR, "Invalid received check path: %s", escaped_path ? escaped_path : ""); free(escaped_path); free(check_path); @@ -1832,6 +1832,7 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { existing/ignore-existing/update/backup/delay-updates policy. */ File* materialized = file_create(check_path); if (materialized && basis.content) { + data_destroy(materialized->data); materialized->data = basis.content; basis.content = NULL; materialized->metadata = file_metadata_create(NULL, &basis.st, false, false); @@ -2095,6 +2096,7 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { file->metadata = meta; file->xattrs = append_xattrs; append_xattrs = NULL; + data_destroy(file->data); file->data = data_create(full, full_size); if (!file->data) { /* data_create already freed full on failure */ file_destroy(file); @@ -2247,6 +2249,7 @@ void dir_time_list_init(DirTimeList* list) { list->entries = NULL; list->count = 0; list->capacity = 0; + list->bytes = 0; } void dir_time_list_free(DirTimeList* list) { @@ -2260,11 +2263,19 @@ void dir_time_list_free(DirTimeList* list) { list->entries = NULL; list->count = 0; list->capacity = 0; + list->bytes = 0; } bool dir_time_list_add(DirTimeList* list, const char* wire_path, const FileMetadata* metadata) { if (!list || !wire_path || !metadata) return true; /* nothing to remember; never a hard error */ + /* Cumulative, not per-frame: the sender may stream a tree across unbounded + STATUS_DIR_TIMES frames, so bound the TOTAL retained here. Reject before + touching the list, leaving it exactly as it was (the caller fails the + transfer, which becomes a clean protocol error). */ + size_t path_len = strlen(wire_path); + if (list->count >= MAX_DIR_TIME_ENTRIES || path_len > MAX_DIR_TIME_BYTES - list->bytes) + return false; if (list->count == list->capacity) { size_t new_capacity = list->capacity == 0 ? 16 : list->capacity * 2; if (new_capacity < list->capacity) @@ -2291,6 +2302,7 @@ bool dir_time_list_add(DirTimeList* list, const char* wire_path, const FileMetad list->paths[list->count] = copy; list->entries[list->count] = *metadata; list->count++; + list->bytes += path_len; return true; } diff --git a/src/shared/file_receive.h b/src/shared/file_receive.h index 37dc15a..ec6ccfa 100644 --- a/src/shared/file_receive.h +++ b/src/shared/file_receive.h @@ -7,6 +7,15 @@ /* Server-side file receive/save path. */ +/* Cumulative caps for the deferred directory-time accumulator. The sender may + * legitimately split a large tree across repeated STATUS_DIR_TIMES frames, so a + * per-frame bound is not enough: the receiver must bound the TOTAL it retains + * against a hostile sender. Mirror the delete-manifest limits + * (MAX_MANIFEST_ENTRIES / MAX_MANIFEST_BYTES): the entry count bounds the + * metadata array and the byte budget bounds the concatenated path strings. */ +#define MAX_DIR_TIME_ENTRIES (1024 * 1024) +#define MAX_DIR_TIME_BYTES (16ULL * 1024 * 1024) + File* file_receive(const Config* config, int file_descriptor); File* file_receive_directory(int file_descriptor, const Config* config); File* file_receive_dir_time(int file_descriptor, const Config* config); @@ -28,6 +37,7 @@ typedef struct { FileMetadata* entries; /* owned, parallel to paths */ size_t count; size_t capacity; + size_t bytes; /* cumulative strlen of every retained path */ } DirTimeList; /* Capture gate shared by the sender-side and receiver-side sinks: directory @@ -39,7 +49,8 @@ bool dir_times_should_capture(const Config* config); void dir_time_list_init(DirTimeList* list); void dir_time_list_free(DirTimeList* list); /* Deep-copy one directory's path + metadata into the list. Returns false on - * allocation failure (the caller fails the transfer). */ + * allocation failure OR when the cumulative entry/byte caps would be exceeded + * (the caller fails the transfer). */ bool dir_time_list_add(DirTimeList* list, const char* wire_path, const FileMetadata* metadata); /* Apply every accumulated directory's mtime (and atime when captured) beneath * `root_directory`, confined fd-relative. Best-effort per entry: an absent diff --git a/tests/test_file.c b/tests/test_file.c index fd715fc..0f0225d 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -1370,6 +1370,76 @@ static void test_dir_time_list() { rmdir(root); } +/* A hostile sender can stream unbounded STATUS_DIR_TIMES frames; the + * accumulator must bound the CUMULATIVE path bytes (not just one frame) and + * reject the add that would cross the cap, leaving the list untouched. */ +static void test_dir_time_list_cap() { + DirTimeList list; + dir_time_list_init(&list); + EXPECT_EQ_INT((int)list.bytes, 0); + FileMetadata metadata = {.mtime_sec = 1, .mtime_nsec = 0}; + + size_t path_len = MAX_STRING_SIZE - 1; + char* path = malloc(path_len + 1); + EXPECT_NOT_NULL(path); + memset(path, 'a', path_len); + path[path_len] = '\0'; + + bool rejected = false; + for (size_t i = 0; i < MAX_DIR_TIME_ENTRIES + 1 && !rejected; i++) { + size_t before_count = list.count; + size_t before_bytes = list.bytes; + if (!dir_time_list_add(&list, path, &metadata)) { + rejected = true; + /* The rejected add must not have partially mutated the list. */ + EXPECT_TRUE(list.count == before_count); + EXPECT_TRUE(list.bytes == before_bytes); + } else { + EXPECT_TRUE(list.count == before_count + 1); + EXPECT_TRUE(list.bytes == before_bytes + path_len); + } + } + EXPECT_TRUE(rejected); + EXPECT_TRUE(list.count <= MAX_DIR_TIME_ENTRIES); + EXPECT_TRUE(list.bytes <= MAX_DIR_TIME_BYTES); + + /* The retained entries are still intact and freeable after the rejection. */ + EXPECT_TRUE(list.count > 0); + EXPECT_TRUE(strcmp(list.paths[0], path) == 0); + dir_time_list_free(&list); + EXPECT_EQ_INT((int)list.bytes, 0); + free(path); +} + +/* receive_incremental_check must reject an empty check_path; every other + * receive path rejects path[0]=='\0'. Feed the check header (empty wire path + * + size/mtime/nsec) and assert the check is refused without being skipped. */ +static void test_receive_incremental_check_empty_path() { + Config* cfg = config_create(); + EXPECT_NOT_NULL(cfg); + cfg->checksum = false; + + int p[2]; + EXPECT_EQ_INT(pipe(p), 0); + size_t wire_len = 0; + unsigned long long check_size = 0; + long long check_mtime = 0; + long long check_mtime_nsec = 0; + EXPECT_TRUE(send_n_data(p[1], &wire_len, sizeof(wire_len))); + EXPECT_TRUE(send_n_data(p[1], &check_size, sizeof(check_size))); + EXPECT_TRUE(send_n_data(p[1], &check_mtime, sizeof(check_mtime))); + EXPECT_TRUE(send_n_data(p[1], &check_mtime_nsec, sizeof(check_mtime_nsec))); + + bool skipped = true; + File* file = receive_incremental_check(p[0], cfg, &skipped); + EXPECT_NULL(file); + EXPECT_FALSE(skipped); + + close(p[0]); + close(p[1]); + config_delete(cfg); +} + /* -K/--keep-dirlinks secure open: with an authorized root, a destination path * component that is a symlink to an IN-ROOT directory is used as that directory * (its referent is opened through a relative O_NOFOLLOW walk from the root fd, @@ -1531,6 +1601,8 @@ void test_file() { } test_file_metadata_create(); test_dir_time_list(); + test_dir_time_list_cap(); + test_receive_incremental_check_empty_path(); test_keep_dirlinks_secure_open(); test_inplace_overwrite_clears_special_mode_bits(); test_inplace_overwrite_metadata_strips_special_bits();