From d102622e2828cd5dda1380062e279bddd454e6d6 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 16 Aug 2026 09:35:26 +0200 Subject: [PATCH] fix: close remaining PR review gaps --- src/client/client_cli.c | 9 +- src/client/client_send.c | 36 +++- src/client/client_send.h | 3 +- src/client/scanner.c | 4 +- src/server/server.c | 77 +++++-- src/shared/chunk.c | 19 +- src/shared/compression.c | 30 ++- src/shared/compression.h | 1 + src/shared/config.c | 141 ++++-------- src/shared/data.c | 4 + src/shared/data.h | 3 + src/shared/delta.c | 130 ++++++----- src/shared/file.c | 402 ++++++++++++++--------------------- src/shared/file.h | 3 +- src/shared/metadata.c | 4 +- src/shared/metadata.h | 2 +- src/shared/multiprocessing.c | 9 +- src/shared/protocol.c | 82 ++++--- src/shared/protocol.h | 3 + src/shared/transport_tls.c | 23 +- src/shared/utils.c | 126 ++++++----- src/shared/utils.h | 4 +- tests/test_array_list.c | 6 + tests/test_compression.c | 2 + tests/test_property.c | 10 + tests/test_queue.c | 6 + tests/test_stress.c | 8 + 27 files changed, 609 insertions(+), 538 deletions(-) diff --git a/src/client/client_cli.c b/src/client/client_cli.c index 0a345a7..7219b5a 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -459,7 +459,6 @@ int main(int argc, char* argv[]) { parse_environment(&env_source, &env_dest, &save_to_disk); int exit_code = 0; - bool config_owned_by_pipeline = false; Config* config = config_create(); if (!config) { fprintf(stderr, "Error: failed to allocate config\n"); @@ -542,18 +541,14 @@ int main(int argc, char* argv[]) { /* Execute transfer */ if (config->use_multithreading) { - config_owned_by_pipeline = true; - exit_code = send_files_multithreaded(config); + exit_code = send_files_multithreaded(&config); } else { exit_code = send_files(config); } cleanup: if (config) { - if (config->log_file) - fclose(config->log_file); - if (!config_owned_by_pipeline) - config_delete(config); + config_delete(config); } return exit_code; } diff --git a/src/client/client_send.c b/src/client/client_send.c index ac2bc09..3cbc5c5 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -70,6 +70,8 @@ static int send_dry_run_manifest(Config* config) { /* Send the delete manifest (list of files) to the server. Returns 0 on success, -1 on failure. */ static int send_delete_manifest(int fd, ArrayList* manifest) { + if (!manifest) + return -1; if (!send_status(fd, STATUS_MANIFEST)) return -1; if (!send_int(fd, manifest->size)) @@ -111,17 +113,22 @@ static int incremental_check(Client* client, File* file, const Config* config, return 1; if (s == STATUS_DELTA_SIGNATURE) { Data* sig_data = receive_data(client->file_descriptor); - if (!sig_data) + if (!sig_data) { + send_status(client->file_descriptor, STATUS_ERROR); return -1; + } DeltaSignature* sig = delta_signature_deserialize(sig_data); data_destroy(sig_data); - if (!sig) + if (!sig) { + send_status(client->file_descriptor, STATUS_ERROR); return -1; + } *out_sig = sig; return 2; } if (s != STATUS_NEXT) { log_message(LOG_LEVEL_ERROR, "Unexpected server status"); + send_status(client->file_descriptor, STATUS_ERROR); return -1; } return 0; @@ -129,8 +136,10 @@ static int incremental_check(Client* client, File* file, const Config* config, static int send_delta(Client* client, File* file, DeltaSignature* sig, Config* config) { Delta* delta = delta_compute(file->data->data, file->data->size, sig, config->delta_block_size); + /* The receiver is blocked after sending the signature. Every local + fallback therefore needs the explicit NEXT response before full data. */ if (!delta) - return 1; + return send_status(client->file_descriptor, STATUS_NEXT) ? 1 : -1; if (!delta_is_worthwhile(delta, file->data->size)) { delta_destroy(delta); @@ -142,14 +151,14 @@ static int send_delta(Client* client, File* file, DeltaSignature* sig, Config* c Data* delta_data = delta_serialize(delta); delta_destroy(delta); if (!delta_data) - return -1; + return send_status(client->file_descriptor, STATUS_NEXT) ? 1 : -1; Data* to_send = delta_data; if (config->use_compression) { to_send = data_compress(delta_data, config->compression_level); data_destroy(delta_data); if (!to_send) - return -1; + return send_status(client->file_descriptor, STATUS_NEXT) ? 1 : -1; } bool ok = send_status(client->file_descriptor, STATUS_DELTA_DATA) && @@ -279,7 +288,8 @@ int send_chunk(Client* client, Chunk* chunk, Config* config) { if (f == NULL) continue; bool stream = f->data->data == NULL && f->data->size > 0; - bool use_sendfile = (config->use_sendfile && !config->use_compression) || stream; + bool use_sendfile = + (config->use_sendfile && !config->use_compression) || (stream && !config->use_compression); int rc = send_single_file(client, f, config, config->use_incremental, use_sendfile); if (rc == 1) continue; @@ -478,7 +488,7 @@ static int load_files_multithreaded(void* pipeline_context) { if (!context->config->use_sendfile) { for (int i = 0; i < chunk->element_count; i++) { File* f = chunk->items[i]; - if (f->data->size > STREAM_THRESHOLD) + if (f->data->size > STREAM_THRESHOLD && !context->config->use_compression) continue; if (!file_load_data(f)) { log_message(LOG_LEVEL_ERROR, "Failed to load file data, skipping"); @@ -630,7 +640,7 @@ int send_files(Config* config) { if (!config->use_sendfile) { for (int i = 0; i < current_chunk->element_count; i++) { File* f = current_chunk->items[i]; - if (f->data->size > STREAM_THRESHOLD) + if (f->data->size > STREAM_THRESHOLD && !config->use_compression) continue; if (!file_load_data(f)) { log_message(LOG_LEVEL_ERROR, "Failed to load file data"); @@ -697,7 +707,10 @@ send_fail: return 1; } -int send_files_multithreaded(Config* config) { +int send_files_multithreaded(Config** config_ptr) { + if (!config_ptr || !*config_ptr) + return 1; + Config* config = *config_ptr; if (config->dry_run) return send_dry_run_manifest(config); @@ -728,8 +741,13 @@ int send_files_multithreaded(Config* config) { queue_destroy(q2); return 1; } + *config_ptr = NULL; /* context now owns config through all remaining paths */ if (config->use_delete) context->manifest = array_list_create(free); + if (config->use_delete && !context->manifest) { + pipeline_context_sender_destroy(context); + return 1; + } thrd_t scanner, loader, sender; bool scanner_created = false; diff --git a/src/client/client_send.h b/src/client/client_send.h index e3c484f..664b060 100644 --- a/src/client/client_send.h +++ b/src/client/client_send.h @@ -7,6 +7,7 @@ int send_chunk(Client* client, Chunk* chunk, Config* config); int send_files(Config* config); -int send_files_multithreaded(Config* config); +/* Takes ownership only when *config is set to NULL on return. */ +int send_files_multithreaded(Config** config); #endif diff --git a/src/client/scanner.c b/src/client/scanner.c index a2ea713..3833edd 100644 --- a/src/client/scanner.c +++ b/src/client/scanner.c @@ -465,8 +465,10 @@ ParallelScanner* parallel_scanner_create(char* root_directory, bool use_metadata if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) continue; char* cur_path = path_cat(root_directory, entry->d_name); - if (!cur_path) + if (!cur_path) { + ps->failed = true; continue; + } struct stat lstats; if (lstat(cur_path, &lstats) != 0) { free(cur_path); diff --git a/src/server/server.c b/src/server/server.c index 44e389e..b46595d 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -36,8 +36,10 @@ static bool tls_client_identity_allowed(SSL* ssl) { char common_name[256]; int length = X509_NAME_get_text_by_NID(X509_get_subject_name(certificate), NID_commonName, common_name, sizeof(common_name)); - bool allowed = length >= 0 && (size_t)length < sizeof(common_name) && - strcmp(common_name, required_client_cn) == 0; + size_t required_length = strlen(required_client_cn); + bool allowed = length >= 0 && (size_t)length == required_length && + required_length < sizeof(common_name) && + memcmp(common_name, required_client_cn, required_length) == 0; X509_free(certificate); return allowed; } @@ -54,19 +56,44 @@ static bool valid_batch_path(const char* path) { static bool __attribute__((unused)) configure_authorization(const char* root) { char resolved[PATH_MAX]; - if (!root || !realpath(root, resolved)) + if (!root) { + file_set_authorized_root(-1, NULL); + utils_set_authorized_root(-1, NULL); return false; + } + int root_fd = open(root, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + if (root_fd < 0) { + file_set_authorized_root(-1, NULL); + utils_set_authorized_root(-1, NULL); + return false; + } + char fd_path[64]; + int fd_path_length = snprintf(fd_path, sizeof(fd_path), "/proc/self/fd/%d", root_fd); + if (fd_path_length < 0 || (size_t)fd_path_length >= sizeof(fd_path) || + !realpath(fd_path, resolved)) { + close(root_fd); + file_set_authorized_root(-1, NULL); + utils_set_authorized_root(-1, NULL); + return false; + } authorized_root = str_dup(resolved); - if (!authorized_root) + if (!authorized_root) { + close(root_fd); + file_set_authorized_root(-1, NULL); + utils_set_authorized_root(-1, NULL); return false; - authorized_root_fd = open(resolved, O_RDONLY | O_DIRECTORY | O_CLOEXEC); - if (authorized_root_fd < 0) { + } + authorized_root_fd = root_fd; + if (!file_set_authorized_root(authorized_root_fd, authorized_root) || + !utils_set_authorized_root(authorized_root_fd, authorized_root)) { + file_set_authorized_root(-1, NULL); + utils_set_authorized_root(-1, NULL); + close(authorized_root_fd); + authorized_root_fd = -1; free(authorized_root); authorized_root = NULL; return false; } - file_set_authorized_root(authorized_root_fd, authorized_root); - utils_set_authorized_root(authorized_root_fd, authorized_root); return true; } @@ -138,6 +165,11 @@ int receive_files(Config* config, int fd) { } struct stat st; char* full_path = path_cat(config->receive_root_directory, check_path); + if (!full_path) { + free(check_path); + send_status(fd, STATUS_ERROR); + return -1; + } bool has_old = full_path && file_stat_secure(full_path, &st); bool match = has_old && (unsigned long long)st.st_size == check_size && (long long)st.st_mtime == check_mtime; @@ -171,8 +203,9 @@ int receive_files(Config* config, int fd) { } if (status == STATUS_MANIFEST) { - if (receive_manifest(fd, config, &status) != 0) + if (receive_manifest(fd, config, &status) != 0) { return -1; + } } if (status != STATUS_FINISHED) { log_message(LOG_LEVEL_ERROR, "Did not receive FINISHED Status"); @@ -209,24 +242,24 @@ void handler(int file_descriptor) { close(file_descriptor); return; } - char resolved_destination[PATH_MAX]; - char* canonical_destination = realpath(config->receive_root_directory, NULL); - const char* destination = - canonical_destination ? canonical_destination : config->receive_root_directory; - if (has_path_traversal(destination) || !path_is_within(authorized_root, destination)) { + char* destination = config->receive_root_directory; + char* joined_destination = NULL; + if (destination && destination[0] != '/') + joined_destination = path_cat(authorized_root, destination); + if (joined_destination) + destination = joined_destination; + if (!destination || has_path_traversal(destination) || + !path_is_within(authorized_root, destination)) { log_message(LOG_LEVEL_ERROR, "Rejected destination outside authorized root"); - free(canonical_destination); + free(joined_destination); config_delete(config); close(file_descriptor); return; } - if (canonical_destination) - snprintf(resolved_destination, sizeof(resolved_destination), "%s", canonical_destination); - else - snprintf(resolved_destination, sizeof(resolved_destination), "%s", destination); - free(canonical_destination); - free(config->receive_root_directory); - config->receive_root_directory = str_dup(resolved_destination); + if (joined_destination) { + free(config->receive_root_directory); + config->receive_root_directory = joined_destination; + } if (!config->receive_root_directory) { config_delete(config); close(file_descriptor); diff --git a/src/shared/chunk.c b/src/shared/chunk.c index bea1a6a..5056c6b 100644 --- a/src/shared/chunk.c +++ b/src/shared/chunk.c @@ -13,6 +13,7 @@ #include "log.h" #include "metadata.h" #include "protocol.h" +#include "utils.h" /* Maximum individual file data size within a chunk (64 MB) */ #define MAX_FILE_DATA_SIZE (64ULL * 1024 * 1024) @@ -82,8 +83,14 @@ static unsigned long long per_file_serialize_size(File* file, bool use_metadata) } Data* chunk_serialize(Chunk* chunk, bool use_metadata) { + if (!chunk || chunk->element_count < 0 || (chunk->element_count > 0 && chunk->items == NULL)) + return NULL; unsigned long long data_size = 0; for (int i = 0; i < chunk->element_count; i++) { + if (!chunk->items[i] || !chunk->items[i]->path || !chunk->items[i]->data || + (chunk->items[i]->data->size > 0 && !chunk->items[i]->data->data) || + chunk->items[i]->path[0] == '\0' || has_path_traversal(chunk->items[i]->path)) + return NULL; unsigned long long file_size = per_file_serialize_size(chunk->items[i], use_metadata); if (file_size == 0 || file_size > ULLONG_MAX - data_size || data_size + file_size > SIZE_MAX) return NULL; @@ -116,6 +123,8 @@ Data* chunk_serialize(Chunk* chunk, bool use_metadata) { } Chunk* chunk_deserialize(Data* data, bool use_metadata) { + if (!data || (!data->data && data->size != 0)) + return NULL; ArrayList* files = array_list_create(file_destroy); if (files == NULL) return NULL; @@ -165,6 +174,12 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { data_pointer += path_len; remaining_size -= path_len; + if (path_len == 0 || has_path_traversal(path)) { + free(path); + array_list_delete(files); + return NULL; + } + File* file = file_create(path); free(path); if (file == NULL) { @@ -285,14 +300,14 @@ Data* chunk_compress(Chunk* chunk, int compression_level, bool use_metadata) { } Chunk* receive_chunk_data(int fd, const Config* config) { - Data* chunk_data = receive_data(fd); + Data* chunk_data = receive_data_limited(fd, MAX_CHUNK_SIZE); if (chunk_data == NULL) { log_message(LOG_LEVEL_ERROR, "Failed to receive chunk data"); return NULL; } Data* data_to_process = chunk_data; if (config->use_compression) { - data_to_process = data_decompress(chunk_data); + data_to_process = data_decompress_limited(chunk_data, MAX_CHUNK_SIZE); data_destroy(chunk_data); if (data_to_process == NULL) { log_message(LOG_LEVEL_ERROR, "Failed to decompress chunk"); diff --git a/src/shared/compression.c b/src/shared/compression.c index 27b6da4..1a42ba5 100644 --- a/src/shared/compression.c +++ b/src/shared/compression.c @@ -2,6 +2,8 @@ #include "data.h" #include "log.h" #include +#include +#include #include #include #include @@ -69,7 +71,10 @@ Data* data_compress(Data* data_to_compress, int compression_level) { return compressed_data; } -Data* data_decompress(Data* compressed_data) { +Data* data_decompress_limited(Data* compressed_data, size_t maximum_size) { + if (!compressed_data || (!compressed_data->data && compressed_data->size != 0) || + maximum_size == 0) + return NULL; log_message(LOG_LEVEL_DEBUG, "Start to decompress data"); unsigned long long dst_size = ZSTD_getFrameContentSize(compressed_data->data, compressed_data->size); @@ -82,15 +87,16 @@ Data* data_decompress(Data* compressed_data) { // ZSTD_CONTENTSIZE_UNKNOWN (~2^64) can cause massive allocation; // fall back to a conservative estimate (3x compressed size) when unknown. if (dst_size == ZSTD_CONTENTSIZE_UNKNOWN) { + if (compressed_data->size > ULLONG_MAX / 3) + return NULL; dst_size = compressed_data->size * 3; if (dst_size < INITIAL_DECOMPRESS_BUF_SIZE) dst_size = INITIAL_DECOMPRESS_BUF_SIZE; - if (dst_size > MAX_DECOMPRESSED_SIZE) - dst_size = MAX_DECOMPRESSED_SIZE; } - if (dst_size > MAX_DECOMPRESSED_SIZE) { - log_message(LOG_LEVEL_ERROR, "Declared decompressed size exceeds %llu bytes", - (unsigned long long)MAX_DECOMPRESSED_SIZE); + unsigned long long hard_limit = + maximum_size < MAX_DECOMPRESSED_SIZE ? maximum_size : MAX_DECOMPRESSED_SIZE; + if (dst_size > hard_limit) { + log_message(LOG_LEVEL_ERROR, "Declared decompressed size exceeds %llu bytes", hard_limit); return NULL; } @@ -101,6 +107,8 @@ Data* data_decompress(Data* compressed_data) { } size_t buf_size = (dst_size > 0) ? (size_t)dst_size : INITIAL_DECOMPRESS_BUF_SIZE; + if (buf_size > maximum_size) + buf_size = maximum_size; Data* uncompressed_data = data_create_empty(buf_size); if (!uncompressed_data) { log_message(LOG_LEVEL_ERROR, "Failed to allocate decompression buffer"); @@ -121,7 +129,7 @@ Data* data_decompress(Data* compressed_data) { return NULL; } if (ret > 0 && output.pos == output.size) { - if (buf_size >= MAX_DECOMPRESSED_SIZE) { + if (buf_size >= hard_limit || buf_size > SIZE_MAX / 2) { log_message(LOG_LEVEL_ERROR, "Decompressed data exceeds %llu bytes", (unsigned long long)MAX_DECOMPRESSED_SIZE); ZSTD_freeDCtx(dctx); @@ -129,8 +137,8 @@ Data* data_decompress(Data* compressed_data) { return NULL; } buf_size *= 2; - if (buf_size > MAX_DECOMPRESSED_SIZE) - buf_size = MAX_DECOMPRESSED_SIZE; + if (buf_size > hard_limit) + buf_size = (size_t)hard_limit; void* new_data = realloc(uncompressed_data->data, buf_size); if (!new_data) { log_message(LOG_LEVEL_ERROR, "Failed to grow decompression buffer"); @@ -150,3 +158,7 @@ Data* data_decompress(Data* compressed_data) { log_message(LOG_LEVEL_DEBUG, "Decompressed data successfully"); return uncompressed_data; } + +Data* data_decompress(Data* compressed_data) { + return data_decompress_limited(compressed_data, MAX_DECOMPRESSED_SIZE); +} diff --git a/src/shared/compression.h b/src/shared/compression.h index d392f25..5322abe 100644 --- a/src/shared/compression.h +++ b/src/shared/compression.h @@ -6,6 +6,7 @@ Data* data_compress(Data* data_to_compress, int compression_level); Data* data_decompress(Data* compressed_data); +Data* data_decompress_limited(Data* compressed_data, size_t maximum_size); bool compression_should_skip(const char* path); #endif diff --git a/src/shared/config.c b/src/shared/config.c index b99303e..38415f1 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -104,6 +104,14 @@ static bool valid_wire_bool(int value) { return value == 0 || value == 1; } +static bool receive_wire_bool(int fd, bool* value) { + int wire_value; + if (!receive_int(fd, &wire_value) || !valid_wire_bool(wire_value)) + return false; + *value = wire_value != 0; + return true; +} + static bool validate_received_config(const Config* config) { return valid_wire_bool(config->save_to_disk) && valid_wire_bool(config->use_multithreading) && valid_wire_bool(config->use_chunk_serialization) && @@ -166,6 +174,10 @@ void config_parse_ssh_dest(Config* config) { void config_delete(Config* config) { if (config == NULL) return; + if (config->log_file) { + fclose(config->log_file); + config->log_file = NULL; + } free(config->version); free(config->send_directory); free(config->receive_root_directory); @@ -347,135 +359,66 @@ Config* config_receive(int file_descriptor) { return NULL; } int tmp; - if (!receive_int(file_descriptor, &tmp)) + if (!receive_wire_bool(file_descriptor, &config->save_to_disk) || + !receive_wire_bool(file_descriptor, &config->use_multithreading) || + !receive_wire_bool(file_descriptor, &config->use_chunk_serialization) || + !receive_wire_bool(file_descriptor, &config->use_compression) || + !receive_wire_bool(file_descriptor, &config->use_metadata)) goto error; - if (!valid_wire_bool(tmp)) - goto error; - config->save_to_disk = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - if (!valid_wire_bool(tmp)) - goto error; - config->use_multithreading = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->use_chunk_serialization = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - if (!valid_wire_bool(tmp)) - goto error; - config->use_compression = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - if (!valid_wire_bool(tmp)) - goto error; - config->use_metadata = tmp; if (!receive_int(file_descriptor, &tmp)) goto error; config->compression_level = tmp; if (!receive_n_data(file_descriptor, &config->chunk_size, sizeof(config->chunk_size))) goto error; - if (!receive_int(file_descriptor, &tmp)) + if (!receive_wire_bool(file_descriptor, &config->use_sendfile) || + !receive_wire_bool(file_descriptor, &config->use_delete) || + !receive_wire_bool(file_descriptor, &config->use_incremental) || + !receive_wire_bool(file_descriptor, &config->use_delta)) goto error; - if (!valid_wire_bool(tmp)) - goto error; - config->use_sendfile = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - if (!valid_wire_bool(tmp)) - goto error; - config->use_delete = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - if (!valid_wire_bool(tmp)) - goto error; - config->use_incremental = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - if (!valid_wire_bool(tmp)) - goto error; - config->use_delta = tmp; if (!receive_n_data(file_descriptor, &config->delta_block_size, sizeof(config->delta_block_size))) goto error; if (!receive_n_data(file_descriptor, &config->delta_max_file_size, sizeof(unsigned long long))) goto error; - if (!receive_int(file_descriptor, &tmp)) + if (!receive_wire_bool(file_descriptor, &config->backup)) goto error; - config->backup = tmp; config->backup_dir = receive_str(file_descriptor); if (config->backup_dir == NULL) goto error; - if (!receive_int(file_descriptor, &tmp)) + if (!receive_wire_bool(file_descriptor, &config->follow_symlinks) || + !receive_wire_bool(file_descriptor, &config->copy_links) || + !receive_wire_bool(file_descriptor, &config->safe_links) || + !receive_wire_bool(file_descriptor, &config->copy_unsafe_links) || + !receive_wire_bool(file_descriptor, &config->preserve_hard_links) || + !receive_wire_bool(file_descriptor, &config->preserve_acls) || + !receive_wire_bool(file_descriptor, &config->preserve_xattrs) || + !receive_wire_bool(file_descriptor, &config->preserve_devices) || + !receive_wire_bool(file_descriptor, &config->preserve_sparse) || + !receive_wire_bool(file_descriptor, &config->update) || + !receive_wire_bool(file_descriptor, &config->inplace) || + !receive_wire_bool(file_descriptor, &config->append) || + !receive_wire_bool(file_descriptor, &config->append_verify) || + !receive_wire_bool(file_descriptor, &config->delete_excluded) || + !receive_wire_bool(file_descriptor, &config->delete_after)) goto error; - config->follow_symlinks = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->copy_links = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->safe_links = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->copy_unsafe_links = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->preserve_hard_links = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->preserve_acls = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->preserve_xattrs = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->preserve_devices = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->preserve_sparse = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->update = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->inplace = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->append = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->append_verify = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->delete_excluded = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->delete_after = tmp; if (!receive_n_data(file_descriptor, &config->max_delete, sizeof(config->max_delete))) goto error; - if (!receive_int(file_descriptor, &tmp)) + if (!receive_wire_bool(file_descriptor, &config->relative) || + !receive_wire_bool(file_descriptor, &config->prune_empty_dirs)) goto error; - config->relative = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->prune_empty_dirs = tmp; config->temp_dir = receive_str(file_descriptor); if (config->temp_dir == NULL) goto error; - if (!receive_int(file_descriptor, &tmp)) + if (!receive_wire_bool(file_descriptor, &config->partial)) goto error; - config->partial = tmp; config->partial_dir = receive_str(file_descriptor); if (config->partial_dir == NULL) goto error; config->suffix = receive_str(file_descriptor); if (config->suffix == NULL) goto error; - if (!receive_int(file_descriptor, &tmp)) + if (!receive_wire_bool(file_descriptor, &config->delete_before) || + !receive_wire_bool(file_descriptor, &config->checksum)) goto error; - config->delete_before = tmp; - if (!receive_int(file_descriptor, &tmp)) - goto error; - config->checksum = tmp; config->compress_choice = receive_str(file_descriptor); if (config->compress_choice == NULL) goto error; diff --git a/src/shared/data.c b/src/shared/data.c index 82e6198..dd43e15 100644 --- a/src/shared/data.c +++ b/src/shared/data.c @@ -21,6 +21,7 @@ Data* data_create_reserve(size_t size) { } d->data = NULL; d->size = size; + d->protocol_charge = 0; return d; } @@ -33,12 +34,15 @@ Data* data_create(void* data, size_t data_size) { } new_data->data = data; new_data->size = data_size; + new_data->protocol_charge = 0; return new_data; } void data_destroy(Data* data) { if (data == NULL) return; + if (data->protocol_charge != 0) + protocol_release_memory(data->protocol_charge); free(data->data); free(data); } diff --git a/src/shared/data.h b/src/shared/data.h index f03fabf..b65ae29 100644 --- a/src/shared/data.h +++ b/src/shared/data.h @@ -6,11 +6,14 @@ typedef struct { void* data; size_t size; + /* Non-zero only for a buffer charged to the protocol connection budget. */ + size_t protocol_charge; } Data; Data* data_create_empty(size_t data_size); Data* data_create_reserve(size_t size); Data* data_create(void* data, size_t data_size); void data_destroy(Data* data); +void protocol_release_memory(size_t charge); #endif diff --git a/src/shared/delta.c b/src/shared/delta.c index 172b526..fc941fb 100644 --- a/src/shared/delta.c +++ b/src/shared/delta.c @@ -1,6 +1,7 @@ #include "delta.h" #include "log.h" #include +#include #include #include @@ -36,7 +37,8 @@ DeltaSignature* delta_signature_create(const void* old_file_data, uint64_t old_f if (old_file_data == NULL || old_file_size == 0 || block_size == 0) return NULL; - if (old_file_size > DELTA_MAX_FILE_SIZE || old_file_size > UINT32_MAX * (uint64_t)block_size) + if (old_file_size > DELTA_MAX_FILE_SIZE || block_size > DELTA_BLOCK_SIZE_MAX || + old_file_size > UINT32_MAX * (uint64_t)block_size) return NULL; uint32_t block_count = (uint32_t)((old_file_size + block_size - 1) / block_size); @@ -48,7 +50,11 @@ DeltaSignature* delta_signature_create(const void* old_file_data, uint64_t old_f sig->file_size = old_file_size; sig->block_size = block_size; sig->block_count = block_count; - sig->blocks = malloc(block_count * sizeof(DeltaBlockSig)); + if (block_count == 0) { + free(sig); + return NULL; + } + sig->blocks = malloc((size_t)block_count * sizeof(DeltaBlockSig)); if (!sig->blocks) { free(sig); return NULL; @@ -70,8 +76,11 @@ Data* delta_signature_serialize(const DeltaSignature* sig) { if (!sig) return NULL; - uint64_t total = sizeof(uint64_t) + sizeof(uint32_t) + sizeof(uint32_t) + - (uint64_t)sig->block_count * (sizeof(uint32_t) + sizeof(uint32_t)); + uint64_t block_bytes = (uint64_t)sig->block_count * (sizeof(uint32_t) + sizeof(uint32_t)); + uint64_t total = sizeof(uint64_t) + sizeof(uint32_t) + sizeof(uint32_t) + block_bytes; + if (block_bytes > UINT64_MAX - (sizeof(uint64_t) + sizeof(uint32_t) + sizeof(uint32_t)) || + total > SIZE_MAX) + return NULL; uint8_t* buf = malloc((size_t)total); if (!buf) @@ -166,8 +175,10 @@ void delta_signature_destroy(DeltaSignature* sig) { static bool ensure_capacity(DeltaInstruction** instrs, uint32_t* capacity, uint32_t count) { if (count < *capacity) return true; + if (*capacity > MAX_DELTA_INSTRUCTIONS / 2) + return false; uint32_t new_cap = *capacity * 2; - DeltaInstruction* tmp = realloc(*instrs, new_cap * sizeof(DeltaInstruction)); + DeltaInstruction* tmp = realloc(*instrs, (size_t)new_cap * sizeof(DeltaInstruction)); if (!tmp) return false; *instrs = tmp; @@ -179,6 +190,8 @@ static bool flush_literal(DeltaInstruction** instrs, uint32_t* capacity, uint32_ const uint8_t* data, uint64_t start, uint64_t end) { if (start >= end) return true; + if (end - start > UINT32_MAX || *count >= MAX_DELTA_INSTRUCTIONS) + return false; uint32_t lit_len = (uint32_t)(end - start); if (!ensure_capacity(instrs, capacity, *count)) return false; @@ -193,16 +206,26 @@ static bool flush_literal(DeltaInstruction** instrs, uint32_t* capacity, uint32_ return true; } +static void free_instructions(DeltaInstruction* instrs, uint32_t count) { + if (!instrs) + return; + for (uint32_t i = 0; i < count; i++) + if (instrs[i].type == DELTA_INSTR_LITERAL) + free(instrs[i].literal.data); + free(instrs); +} + Delta* delta_compute(const void* new_file_data, uint64_t new_file_size, const DeltaSignature* sig, uint32_t block_size) { - if (!new_file_data || !sig || new_file_size == 0 || block_size == 0) + if (!new_file_data || !sig || !sig->blocks || new_file_size == 0 || block_size == 0 || + block_size > DELTA_BLOCK_SIZE_MAX || sig->block_size != block_size) return NULL; const uint8_t* new_data = (const uint8_t*)new_file_data; uint32_t capacity = 64; uint32_t count = 0; - DeltaInstruction* instrs = malloc(capacity * sizeof(DeltaInstruction)); + DeltaInstruction* instrs = malloc((size_t)capacity * sizeof(DeltaInstruction)); if (!instrs) return NULL; @@ -246,14 +269,14 @@ Delta* delta_compute(const void* new_file_data, uint64_t new_file_size, const De if (xxh == sig->blocks[j].xxhash) { if (has_literal) { if (!flush_literal(&instrs, &capacity, &count, new_data, literal_start, i)) { - free(instrs); + free_instructions(instrs, count); return NULL; } has_literal = false; } if (!ensure_capacity(&instrs, &capacity, count)) { - free(instrs); + free_instructions(instrs, count); return NULL; } instrs[count].type = DELTA_INSTR_BLOCK_MATCH; @@ -281,18 +304,14 @@ Delta* delta_compute(const void* new_file_data, uint64_t new_file_size, const De if (has_literal) { if (!flush_literal(&instrs, &capacity, &count, new_data, literal_start, new_file_size)) { - free(instrs); + free_instructions(instrs, count); return NULL; } } Delta* delta = malloc(sizeof(Delta)); if (!delta) { - for (uint32_t k = 0; k < count; k++) { - if (instrs[k].type == DELTA_INSTR_LITERAL) - free(instrs[k].literal.data); - } - free(instrs); + free_instructions(instrs, count); return NULL; } @@ -302,11 +321,24 @@ Delta* delta_compute(const void* new_file_data, uint64_t new_file_size, const De delta->delta_size = 0; for (uint32_t k = 0; k < count; k++) { + if (delta->delta_size == UINT64_MAX) { + delta_destroy(delta); + return NULL; + } delta->delta_size += 1; if (instrs[k].type == DELTA_INSTR_BLOCK_MATCH) { + if (delta->delta_size > UINT64_MAX - sizeof(uint32_t) * 3) { + delta_destroy(delta); + return NULL; + } delta->delta_size += sizeof(uint32_t) * 3; } else { - delta->delta_size += sizeof(uint32_t) + instrs[k].literal.length; + uint64_t extra = sizeof(uint32_t) + instrs[k].literal.length; + if (delta->delta_size > UINT64_MAX - extra) { + delta_destroy(delta); + return NULL; + } + delta->delta_size += extra; } } @@ -317,7 +349,12 @@ Data* delta_serialize(const Delta* delta) { if (!delta) return NULL; - uint64_t total = sizeof(uint64_t) + sizeof(uint32_t) + delta->delta_size; + if (delta->instruction_count > 0 && !delta->instructions) + return NULL; + uint64_t header_size = sizeof(uint64_t) + sizeof(uint32_t); + if (delta->delta_size > UINT64_MAX - header_size || header_size + delta->delta_size > SIZE_MAX) + return NULL; + uint64_t total = header_size + delta->delta_size; uint8_t* buf = malloc((size_t)total); if (!buf) return NULL; @@ -375,8 +412,10 @@ Delta* delta_deserialize(const Data* data) { return NULL; } - delta->instructions = malloc(delta->instruction_count * sizeof(DeltaInstruction)); - if (!delta->instructions) { + delta->instructions = delta->instruction_count == 0 + ? NULL + : malloc((size_t)delta->instruction_count * sizeof(DeltaInstruction)); + if (delta->instruction_count > 0 && !delta->instructions) { free(delta); return NULL; } @@ -385,11 +424,7 @@ Delta* delta_deserialize(const Data* data) { for (uint32_t i = 0; i < delta->instruction_count; i++) { if (pos >= data->size) { - for (uint32_t k = 0; k < i; k++) { - if (delta->instructions[k].type == DELTA_INSTR_LITERAL) - free(delta->instructions[k].literal.data); - } - free(delta->instructions); + free_instructions(delta->instructions, i); free(delta); return NULL; } @@ -401,12 +436,8 @@ Delta* delta_deserialize(const Data* data) { delta->delta_size += 1; if (type == DELTA_OP_BLOCK_MATCH) { - if (pos + sizeof(uint32_t) * 3 > data->size) { - for (uint32_t k = 0; k < i; k++) { - if (delta->instructions[k].type == DELTA_INSTR_LITERAL) - free(delta->instructions[k].literal.data); - } - free(delta->instructions); + if (data->size - pos < sizeof(uint32_t) * 3) { + free_instructions(delta->instructions, i); free(delta); return NULL; } @@ -419,12 +450,8 @@ Delta* delta_deserialize(const Data* data) { pos += sizeof(uint32_t); delta->delta_size += sizeof(uint32_t) * 3; } else if (type == DELTA_OP_LITERAL) { - if (pos + sizeof(uint32_t) > data->size) { - for (uint32_t k = 0; k < i; k++) { - if (delta->instructions[k].type == DELTA_INSTR_LITERAL) - free(delta->instructions[k].literal.data); - } - free(delta->instructions); + if (data->size - pos < sizeof(uint32_t)) { + free_instructions(delta->instructions, i); free(delta); return NULL; } @@ -433,23 +460,15 @@ Delta* delta_deserialize(const Data* data) { pos += sizeof(uint32_t); uint32_t lit_len = delta->instructions[i].literal.length; - if (pos + lit_len > data->size) { - for (uint32_t k = 0; k < i; k++) { - if (delta->instructions[k].type == DELTA_INSTR_LITERAL) - free(delta->instructions[k].literal.data); - } - free(delta->instructions); + if (lit_len > data->size - pos) { + free_instructions(delta->instructions, i); free(delta); return NULL; } - delta->instructions[i].literal.data = malloc(lit_len); + delta->instructions[i].literal.data = malloc(lit_len ? lit_len : 1); if (!delta->instructions[i].literal.data) { log_message(LOG_LEVEL_ERROR, "Failed to allocate %u bytes for literal data", lit_len); - for (uint32_t k = 0; k < i; k++) { - if (delta->instructions[k].type == DELTA_INSTR_LITERAL) - free(delta->instructions[k].literal.data); - } - free(delta->instructions); + free_instructions(delta->instructions, i); free(delta); return NULL; } @@ -457,11 +476,7 @@ Delta* delta_deserialize(const Data* data) { pos += lit_len; delta->delta_size += sizeof(uint32_t) + lit_len; } else { - for (uint32_t k = 0; k < i; k++) { - if (delta->instructions[k].type == DELTA_INSTR_LITERAL) - free(delta->instructions[k].literal.data); - } - free(delta->instructions); + free_instructions(delta->instructions, i); free(delta); return NULL; } @@ -477,7 +492,7 @@ void* delta_apply(const void* old_data, uint64_t old_size, const Delta* delta, delta->new_file_size > DELTA_MAX_FILE_SIZE || delta->new_file_size > SIZE_MAX) return NULL; - void* output = malloc((size_t)delta->new_file_size); + void* output = malloc(delta->new_file_size ? (size_t)delta->new_file_size : 1); if (!output) return NULL; @@ -502,7 +517,7 @@ void* delta_apply(const void* old_data, uint64_t old_size, const Delta* delta, } memcpy(out + out_pos, old + src_offset, len); out_pos += len; - } else { + } else if (delta->instructions[i].type == DELTA_INSTR_LITERAL) { uint32_t len = delta->instructions[i].literal.length; if (out_pos > delta->new_file_size || (uint64_t)len > delta->new_file_size - out_pos) { free(output); @@ -510,6 +525,9 @@ void* delta_apply(const void* old_data, uint64_t old_size, const Delta* delta, } memcpy(out + out_pos, delta->instructions[i].literal.data, len); out_pos += len; + } else { + free(output); + return NULL; } } @@ -545,7 +563,7 @@ bool delta_should_attempt(uint64_t old_size, uint64_t new_size, uint64_t max_fil } bool delta_is_worthwhile(const Delta* delta, uint64_t new_file_size) { - if (!delta || delta->instruction_count == 0) + if (!delta || delta->instruction_count == 0 || new_file_size == 0) return false; bool has_match = false; diff --git a/src/shared/file.c b/src/shared/file.c index 0575655..ecc0f96 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -24,7 +24,7 @@ #include "utils.h" #define MAX_SERVER_DELETE_COUNT 100000U -#define MAX_FILE_DATA_SIZE (64ULL * 1024 * 1024) +#define MAX_FILE_DATA_SIZE MAX_RECEIVE_FILE_SIZE bool file_checksum(File* file, uint64_t* checksum) { if (!file || !checksum || !file->data) @@ -48,7 +48,7 @@ File* file_create(const char* path) { return NULL; } - int path_len = strlen(path); + size_t path_len = strlen(path); file->path = (char*)malloc(path_len + 1); if (file->path == NULL) { free(file); @@ -104,7 +104,7 @@ void file_metadata_destroy(void* metadata) { } bool file_load_data(File* file) { - if (file == NULL) + if (file == NULL || !file->data) return false; if (file->data->data == NULL) { if (file->data->size == 0) @@ -128,7 +128,7 @@ bool file_load_data(File* file) { bool file_send_single_calls(File* file, int file_descriptor, bool use_metadata, int compression_level, bool send_path) { - if (!file || !file->path || !file->data) + if (!file || !file->path || !file->data || (file->data->size != 0 && !file->data->data)) return false; const Data* data_to_send = file->data; Data* compressed_data = NULL; @@ -159,6 +159,7 @@ bool file_send_single_calls(File* file, int file_descriptor, bool use_metadata, static bool to_disk_secure(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse, const FileMetadata* metadata); static int open_secure_parent(const char* path, char** leaf_out, bool create_dirs); +static bool ensure_directory_secure(const char* path); bool file_path_exists_secure(const char* path) { struct stat st; @@ -184,15 +185,23 @@ static bool rename_secure(const char* old_path, const char* new_path); static int authorized_root_fd = -1; static char* authorized_root_path; -void file_set_authorized_root(int fd, const char* canonical_path) { - authorized_root_fd = fd; - free(authorized_root_path); - authorized_root_path = canonical_path ? str_dup(canonical_path) : NULL; +static bool path_is_within_root(const char* root, const char* path) { + size_t root_len = strlen(root); + return strncmp(root, path, root_len) == 0 && (path[root_len] == '\0' || path[root_len] == '/'); } -static bool path_is_within_root(const char* root, const char* path) { - size_t n = strlen(root); - return strncmp(root, path, n) == 0 && (path[n] == '\0' || path[n] == '/'); +bool file_set_authorized_root(int fd, const char* canonical_path) { + char* path_copy = canonical_path ? str_dup(canonical_path) : NULL; + if (canonical_path && !path_copy) { + authorized_root_fd = -1; + free(authorized_root_path); + authorized_root_path = NULL; + return false; + } + authorized_root_fd = fd; + free(authorized_root_path); + authorized_root_path = path_copy; + return true; } bool file_save_to_disk(const char* root_directory, const File* file, const Config* config) { @@ -202,9 +211,11 @@ bool file_save_to_disk(const char* root_directory, const File* file, const Confi const char* backup_suffix = (config && config->suffix) ? config->suffix : "~"; const char* backup_dir = (config && config->backup_dir) ? config->backup_dir : NULL; const char* partial_dir = (config && config->partial_dir) ? config->partial_dir : NULL; - char *confined_backup = NULL, *confined_partial = NULL; + char *confined_backup = NULL, *confined_partial = NULL, *disk_path = NULL; + char *backup_path = NULL, *parent_copy = NULL; - if (!file || !file->path || !file->data || has_path_traversal(file->path) || + if (!file || !file->path || !file->data || (file->data->size != 0 && !file->data->data) || + has_path_traversal(file->path) || (backup_enabled && (!backup_suffix || backup_suffix[0] == '\0' || strchr(backup_suffix, '/') != NULL || strcmp(backup_suffix, ".") == 0 || strcmp(backup_suffix, "..") == 0))) { @@ -224,45 +235,20 @@ bool file_save_to_disk(const char* root_directory, const File* file, const Confi return false; } - char* resolved_root = NULL; const char* actual_root = (partial_dir && config && config->partial) ? confined_partial : root_directory; - resolved_root = realpath(actual_root, NULL); - if (resolved_root == NULL) { - if (mkdir_r(actual_root)) { - resolved_root = realpath(actual_root, NULL); - } - } - if (resolved_root == NULL) { - log_message(LOG_LEVEL_ERROR, "Failed to resolve destination root: %s", actual_root); - free(confined_backup); - free(confined_partial); - return false; - } - char* resolved_base = realpath(root_directory, NULL); - if (resolved_base == NULL || !path_is_within_root(resolved_base, resolved_root)) { - free(resolved_base); - free(confined_backup); - free(confined_partial); - free(resolved_root); - return false; - } - free(resolved_base); - - char* disk_path = path_cat(resolved_root, file->path); + disk_path = path_cat(actual_root, file->path); if (disk_path == NULL) { free(confined_backup); free(confined_partial); - free(resolved_root); return false; } /* --update is receiver-side policy: never replace a newer destination. */ if (config && config->update) { struct stat destination_stat; - if (stat(disk_path, &destination_stat) == 0 && file->metadata && + if (file_stat_secure(disk_path, &destination_stat) && file->metadata && destination_stat.st_mtime > file->metadata->mtime_sec) { - free(resolved_root); free(confined_backup); free(confined_partial); free(disk_path); @@ -272,99 +258,50 @@ bool file_save_to_disk(const char* root_directory, const File* file, const Confi if (backup_enabled) { struct stat backup_stat; - if (stat(disk_path, &backup_stat) == 0) { - char* backup_path = NULL; + if (file_stat_secure(disk_path, &backup_stat)) { if (backup_dir) { - char* resolved_backup_dir = realpath(confined_backup, NULL); - if (!resolved_backup_dir) { - mkdir_r(confined_backup); - resolved_backup_dir = realpath(confined_backup, NULL); - } - if (resolved_backup_dir) { - char* backup_base = realpath(root_directory, NULL); - if (backup_base && path_is_within_root(backup_base, resolved_backup_dir)) - backup_path = path_cat(resolved_backup_dir, file->path); - free(backup_base); - free(resolved_backup_dir); - } - } - if (!backup_path) { + backup_path = path_cat(confined_backup, file->path); + } else { size_t path_len = strlen(disk_path); size_t suffix_len = strlen(backup_suffix); + if (path_len > SIZE_MAX - suffix_len - 1) + goto fail; backup_path = malloc(path_len + suffix_len + 1); if (backup_path) { memcpy(backup_path, disk_path, path_len); memcpy(backup_path + path_len, backup_suffix, suffix_len + 1); } } - if (backup_path) { - char* backup_dir_path = str_dup(backup_path); - if (backup_dir_path) { - const char* bdir = dirname(backup_dir_path); - mkdir_r(bdir); - free(backup_dir_path); - } - if (!rename_secure(disk_path, backup_path)) { - free(backup_path); - free(resolved_root); - free(confined_backup); - free(confined_partial); - free(disk_path); - return false; - } - free(backup_path); - } + if (!backup_path) + goto fail; + parent_copy = str_dup(backup_path); + if (!parent_copy || !ensure_directory_secure(dirname(parent_copy))) + goto fail; + free(parent_copy); + parent_copy = NULL; + if (!rename_secure(disk_path, backup_path)) + goto fail; + free(backup_path); + backup_path = NULL; } } - char* dir_dup = str_dup(disk_path); - if (!dir_dup) { - free(confined_backup); - free(confined_partial); - free(resolved_root); - free(disk_path); - return false; - } - char* dir_str = dirname(dir_dup); - if (!mkdir_r(dir_str)) { - free(dir_dup); - free(confined_backup); - free(confined_partial); - free(resolved_root); - free(disk_path); - return false; - } - char* resolved_dir = realpath(dir_str, NULL); - free(dir_dup); - if (resolved_dir == NULL) { - log_message(LOG_LEVEL_ERROR, "Failed to resolve directory for: %s", disk_path); - free(confined_backup); - free(confined_partial); - free(resolved_root); - free(disk_path); - return false; - } - - size_t root_len = strlen(resolved_root); - if (strncmp(resolved_dir, resolved_root, root_len) != 0 || - (resolved_dir[root_len] != '\0' && resolved_dir[root_len] != '/')) { - log_message(LOG_LEVEL_ERROR, "Path escape detected: %s is outside %s", disk_path, actual_root); - free(resolved_dir); - free(confined_backup); - free(confined_partial); - free(resolved_root); - free(disk_path); - return false; - } - free(resolved_dir); - free(resolved_root); - bool ok = to_disk_secure(disk_path, file->data->data, file->data->size, inplace, sparse, file->metadata); + free(parent_copy); + free(backup_path); free(confined_backup); free(confined_partial); free(disk_path); return ok; + +fail: + free(parent_copy); + free(backup_path); + free(confined_backup); + free(confined_partial); + free(disk_path); + return false; } static File* receive_delta_file(int fd, const Config* config, const char* check_path, @@ -402,7 +339,7 @@ static File* receive_delta_file(int fd, const Config* config, const char* check_ } if (resp == STATUS_DELTA_DATA) { - Data* delta_data = receive_data(fd); + Data* delta_data = receive_data_limited(fd, MAX_RECEIVE_FILE_SIZE); if (!delta_data) { delta_signature_destroy(sig); free(old_data); @@ -412,7 +349,7 @@ static File* receive_delta_file(int fd, const Config* config, const char* check_ Data* raw_delta = delta_data; if (config->use_compression) { - raw_delta = data_decompress(delta_data); + raw_delta = data_decompress_limited(delta_data, MAX_RECEIVE_FILE_SIZE); data_destroy(delta_data); if (!raw_delta) { free(old_data); @@ -431,8 +368,15 @@ static File* receive_delta_file(int fd, const Config* config, const char* check_ return NULL; } - void* new_data = delta_apply(old_data, old_size, delta, config->delta_block_size); uint64_t new_size = delta->new_file_size; + if (new_size > MAX_RECEIVE_FILE_SIZE || new_size > SIZE_MAX) { + delta_destroy(delta); + free(old_data); + delta_signature_destroy(sig); + send_status(fd, STATUS_ERROR); + return NULL; + } + void* new_data = delta_apply(old_data, old_size, delta, config->delta_block_size); delta_destroy(delta); if (!new_data) { @@ -500,7 +444,7 @@ static File* receive_delta_file(int fd, const Config* config, const char* check_ } } - Data* file_data = receive_data(fd); + Data* file_data = receive_data_limited(fd, MAX_RECEIVE_FILE_SIZE); if (file_data == NULL) { file_destroy(file); send_status(fd, STATUS_ERROR); @@ -508,7 +452,7 @@ static File* receive_delta_file(int fd, const Config* config, const char* check_ } if (config->use_compression) { - Data* uncompressed = data_decompress(file_data); + Data* uncompressed = data_decompress_limited(file_data, MAX_RECEIVE_FILE_SIZE); data_destroy(file_data); if (uncompressed == NULL) { file_destroy(file); @@ -536,6 +480,10 @@ static File* receive_delta_file(int fd, const Config* config, const char* check_ } File* receive_incremental_check(int fd, const Config* config, bool* skipped) { + if (!config || !skipped) { + send_status(fd, STATUS_ERROR); + return NULL; + } *skipped = false; char* check_path = receive_str(fd); if (check_path == NULL) { @@ -558,6 +506,12 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { return NULL; } + if (check_size > MAX_RECEIVE_FILE_SIZE) { + free(check_path); + send_status(fd, STATUS_ERROR); + return NULL; + } + if (has_path_traversal(check_path)) { log_message(LOG_LEVEL_ERROR, "Path traversal detected: %s", check_path); free(check_path); @@ -566,22 +520,25 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { } char* full_path = path_cat(config->receive_root_directory, check_path); + if (!full_path) { + free(check_path); + send_status(fd, STATUS_ERROR); + return NULL; + } struct stat st; bool has_old_file = false; int old_fd = -1; - if (full_path) { - char* leaf = NULL; - int parent_fd = open_secure_parent(full_path, &leaf, false); - if (parent_fd >= 0) { - old_fd = openat(parent_fd, leaf, O_RDONLY | O_CLOEXEC | O_NOFOLLOW); - free(leaf); - close(parent_fd); - has_old_file = old_fd >= 0 && fstat(old_fd, &st) == 0 && S_ISREG(st.st_mode); - } + char* leaf = NULL; + int parent_fd = open_secure_parent(full_path, &leaf, false); + if (parent_fd >= 0) { + old_fd = openat(parent_fd, leaf, O_RDONLY | O_CLOEXEC | O_NOFOLLOW); + free(leaf); + close(parent_fd); + has_old_file = old_fd >= 0 && fstat(old_fd, &st) == 0 && S_ISREG(st.st_mode); } unsigned long long old_size = has_old_file ? (unsigned long long)st.st_size : 0; void* old_data = NULL; - if (has_old_file && old_size > 0 && old_size <= DELTA_MAX_FILE_SIZE && old_size <= SIZE_MAX) { + if (has_old_file && old_size > 0 && old_size <= MAX_RECEIVE_FILE_SIZE && old_size <= SIZE_MAX) { old_data = malloc((size_t)old_size); if (old_data) { size_t got = 0; @@ -625,7 +582,7 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { return NULL; } - bool try_delta = config->use_delta && has_old_file && + bool try_delta = config->use_delta && has_old_file && old_data != NULL && delta_should_attempt(old_size, check_size, config->delta_max_file_size); if (try_delta) { @@ -667,7 +624,7 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { } } - Data* file_data = receive_data(fd); + Data* file_data = receive_data_limited(fd, MAX_RECEIVE_FILE_SIZE); if (file_data == NULL) { file_destroy(file); send_status(fd, STATUS_ERROR); @@ -675,7 +632,7 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { } if (config->use_compression) { - Data* uncompressed = data_decompress(file_data); + Data* uncompressed = data_decompress_limited(file_data, MAX_RECEIVE_FILE_SIZE); data_destroy(file_data); if (uncompressed == NULL) { file_destroy(file); @@ -708,9 +665,19 @@ static int open_secure_parent(const char* path, char** leaf_out, bool create_dir return -1; } int fd; - if (authorized_root_fd >= 0 && authorized_root_path && path[0] == '/' && - path_is_within_root(authorized_root_path, path)) { + if (authorized_root_fd >= 0) { + if (!authorized_root_path || path[0] != '/' || + !path_is_within_root(authorized_root_path, path)) { + free(copy); + free(leaf); + return -1; + } fd = dup(authorized_root_fd); + if (fd < 0) { + free(copy); + free(leaf); + return -1; + } size_t root_len = strlen(authorized_root_path); char* relative = str_dup(path + root_len); if (!relative) { @@ -734,10 +701,18 @@ static int open_secure_parent(const char* path, char** leaf_out, bool create_dir char* save = NULL; char* component = strtok_r(parent, "/", &save); while (component) { - if (strcmp(component, ".") != 0 && strcmp(component, "..") != 0) { + if (strcmp(component, "..") == 0) { + close(fd); + free(copy); + free(leaf); + return -1; + } + if (strcmp(component, ".") != 0) { int next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); - if (create_dirs && next < 0 && errno == ENOENT && mkdirat(fd, component, 0755) == 0) - next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + if (create_dirs && next < 0 && errno == ENOENT) { + if (mkdirat(fd, component, 0755) == 0 || errno == EEXIST) + next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + } if (next < 0) { close(fd); free(copy); @@ -754,9 +729,28 @@ static int open_secure_parent(const char* path, char** leaf_out, bool create_dir return fd; } +static bool ensure_directory_secure(const char* path) { + char* leaf = NULL; + int parent_fd = open_secure_parent(path, &leaf, true); + if (parent_fd < 0) + return false; + + int dir_fd = openat(parent_fd, leaf, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + if (dir_fd < 0 && errno == ENOENT) { + if (mkdirat(parent_fd, leaf, 0755) == 0 || errno == EEXIST) + dir_fd = openat(parent_fd, leaf, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + } + bool ok = dir_fd >= 0; + if (dir_fd >= 0) + close(dir_fd); + close(parent_fd); + free(leaf); + return ok; +} + static bool rename_secure(const char* old_path, const char* new_path) { char *old_leaf = NULL, *new_leaf = NULL; - int old_parent = open_secure_parent(old_path, &old_leaf, true); + int old_parent = open_secure_parent(old_path, &old_leaf, false); int new_parent = open_secure_parent(new_path, &new_leaf, true); bool ok = old_parent >= 0 && new_parent >= 0 && renameat(old_parent, old_leaf, new_parent, new_leaf) == 0; @@ -833,106 +827,6 @@ bool to_disk(const char* path, const void* data, unsigned long long data_size, b if (!path || (!data && data_size != 0) || has_path_traversal(path)) return false; return to_disk_secure(path, data, data_size, inplace, sparse, NULL); - /* Kept below only as historical context; all writes use descriptor-relative operations. */ - char* tmp_path = NULL; - char* directory = NULL; - - char* path_dup = str_dup(path); - if (!path_dup) - return false; - const char* dir_result = dirname(path_dup); - directory = str_dup(dir_result); - free(path_dup); - if (!directory) - return false; - - bool ok = true; - if (!mkdir_r(directory)) - goto done; - - if (inplace) { - FILE* file_pointer = fopen(path, "wb"); - if (file_pointer == NULL) { - perror("Could not open file for inplace write"); - ok = false; - goto done; - } - if (sparse && data_size > 0) { - if (fseek(file_pointer, data_size - 1, SEEK_SET) != 0) { - perror("Failed to seek for sparse file"); - fclose(file_pointer); - ok = false; - goto done; - } - if (fwrite("", 1, 1, file_pointer) != 1) { - perror("Failed to write sparse file"); - fclose(file_pointer); - ok = false; - goto done; - } - rewind(file_pointer); - } - if (data_size > 0 && fwrite(data, 1, data_size, file_pointer) != data_size) { - perror("Failed to write all data to file"); - fclose(file_pointer); - ok = false; - goto done; - } - fclose(file_pointer); - free(directory); - return true; - } - - size_t path_len = strlen(path); - tmp_path = malloc(path_len + 5); - if (!tmp_path) { - ok = false; - goto done; - } - memcpy(tmp_path, path, path_len); - memcpy(tmp_path + path_len, ".tmp", 5); - - FILE* file_pointer = fopen(tmp_path, "wb"); - if (file_pointer == NULL) { - perror("Could not open temporary file"); - ok = false; - goto done; - } - if (sparse && data_size > 0) { - if (fseek(file_pointer, data_size - 1, SEEK_SET) != 0) { - perror("Failed to seek for sparse file"); - fclose(file_pointer); - ok = false; - goto done; - } - if (fwrite("", 1, 1, file_pointer) != 1) { - perror("Failed to write sparse file"); - fclose(file_pointer); - ok = false; - goto done; - } - rewind(file_pointer); - } - if (fwrite(data, 1, data_size, file_pointer) != data_size) { - perror("Failed to write all data to temporary file"); - fclose(file_pointer); - unlink(tmp_path); - ok = false; - goto done; - } - fclose(file_pointer); - - if (rename(tmp_path, path) != 0) { - perror("Failed to atomically rename temporary file"); - unlink(tmp_path); - ok = false; - goto done; - } - -done: - free(tmp_path); - free(directory); - return ok; } bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int compression_level, @@ -1044,13 +938,13 @@ File* file_receive(const Config* config, int file_descriptor) { return NULL; } } - Data* file_data = receive_data(file_descriptor); + Data* file_data = receive_data_limited(file_descriptor, MAX_RECEIVE_FILE_SIZE); if (file_data == NULL) { file_destroy(file); return NULL; } - if (config->use_compression) { - Data* file_data_uncompressed = data_decompress(file_data); + if (config->use_compression && !compression_should_skip(file->path)) { + Data* file_data_uncompressed = data_decompress_limited(file_data, MAX_RECEIVE_FILE_SIZE); data_destroy(file_data); if (file_data_uncompressed == NULL) { file_destroy(file); @@ -1069,6 +963,8 @@ File* file_receive(const Config* config, int file_descriptor) { } size_t file_content_to_buffer(File* file) { + if (!file || !file->path || !file->data || (!file->data->data && file->data->size != 0)) + return 0; FILE* file_pointer = fopen(file->path, "rb"); if (file_pointer == NULL) { perror("Could not open the file!"); @@ -1085,16 +981,26 @@ size_t file_content_to_buffer(File* file) { } int receive_manifest(int fd, const Config* config, int* next_status) { + if (!config) { + send_status(fd, STATUS_ERROR); + return -1; + } int received_status = STATUS_ERROR; int* status_out = next_status ? next_status : &received_status; int count; - if (!receive_int(fd, &count)) + if (!receive_int(fd, &count)) { + send_status(fd, STATUS_ERROR); return -1; - if (count < 0 || count > MAX_MANIFEST_ENTRIES) + } + if (count < 0 || count > MAX_MANIFEST_ENTRIES) { + send_status(fd, STATUS_ERROR); return -1; + } ArrayList* manifest = array_list_create(free); - if (!manifest) + if (!manifest) { + send_status(fd, STATUS_ERROR); return -1; + } size_t manifest_bytes = 0; for (int i = 0; i < count; i++) { char* s = receive_str(fd); @@ -1104,22 +1010,28 @@ int receive_manifest(int fd, const Config* config, int* next_status) { (manifest_bytes += entry_size) > MAX_MANIFEST_BYTES || !array_list_add(manifest, s)) { free(s); array_list_delete(manifest); + send_status(fd, STATUS_ERROR); return -1; } } if (!receive_status(fd, status_out)) { array_list_delete(manifest); + send_status(fd, STATUS_ERROR); return -1; } /* Deletion is a commit operation: never perform it until the sender has completed the manifest frame successfully. */ if (*status_out != STATUS_FINISHED || !config->use_delete) { array_list_delete(manifest); + if (*status_out != STATUS_FINISHED) + send_status(fd, STATUS_ERROR); return *status_out == STATUS_FINISHED ? 0 : -1; } fprintf(stderr, "Deleting files not in manifest...\n"); bool deletion_ok = delete_extras_limited(config->receive_root_directory, manifest, MAX_SERVER_DELETE_COUNT); array_list_delete(manifest); + if (!deletion_ok) + send_status(fd, STATUS_ERROR); return deletion_ok ? 0 : -1; } diff --git a/src/shared/file.h b/src/shared/file.h index b103bfe..e264614 100644 --- a/src/shared/file.h +++ b/src/shared/file.h @@ -38,7 +38,8 @@ void file_metadata_destroy(void* metadata); bool to_disk(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse); bool file_save_to_disk(const char* root_directory, const File* file, const Config* config); -void file_set_authorized_root(int fd, const char* canonical_path); +/* A configured fd without a canonical identity deliberately rejects paths. */ +bool file_set_authorized_root(int fd, const char* canonical_path); File* receive_incremental_check(int fd, const Config* config, bool* skipped); bool file_path_exists_secure(const char* path); bool file_stat_secure(const char* path, struct stat* st); diff --git a/src/shared/metadata.c b/src/shared/metadata.c index 2dbb7af..0c37309 100644 --- a/src/shared/metadata.c +++ b/src/shared/metadata.c @@ -51,6 +51,8 @@ FileMetadata* metadata_from_buf(char** buf) { int32_t present; memcpy(&present, *buf, sizeof(present)); *buf += sizeof(present); + if (present != 0 && present != 1) + return NULL; if (!present) return NULL; FileMetadata* m = malloc(sizeof(FileMetadata)); @@ -84,7 +86,7 @@ FileMetadata* metadata_from_buf(char** buf) { return m; } -bool metadata_send(int file_descriptor, FileMetadata* m) { +bool metadata_send(int file_descriptor, const FileMetadata* m) { if (m == NULL) { int32_t zero = 0; return send_n_data(file_descriptor, &zero, sizeof(zero)); diff --git a/src/shared/metadata.h b/src/shared/metadata.h index f6f90aa..b8d7ba9 100644 --- a/src/shared/metadata.h +++ b/src/shared/metadata.h @@ -27,7 +27,7 @@ void metadata_to_buf(char** buf, const FileMetadata* m); FileMetadata* metadata_from_buf(char** buf); -bool metadata_send(int file_descriptor, FileMetadata* m); +bool metadata_send(int file_descriptor, const FileMetadata* m); FileMetadata* metadata_receive(int file_descriptor, int* ok); void file_restore_metadata(const char* path, const FileMetadata* metadata); bool file_restore_metadata_fd(int fd, const FileMetadata* metadata); diff --git a/src/shared/multiprocessing.c b/src/shared/multiprocessing.c index 09f38e5..f3c6536 100644 --- a/src/shared/multiprocessing.c +++ b/src/shared/multiprocessing.c @@ -233,6 +233,12 @@ int receive_thread(void* pipeline_context) { RECEIVE_THREAD_FAIL(); } char* full_path = path_cat(config->receive_root_directory, check_path); + if (!full_path) { + free(check_path); + if (!send_status(file_descriptor, STATUS_ERROR)) + RECEIVE_THREAD_FAIL(); + RECEIVE_THREAD_FAIL(); + } struct stat st; bool has_old = full_path && file_stat_secure(full_path, &st); bool match = has_old && (unsigned long long)st.st_size == check_size && @@ -263,8 +269,9 @@ int receive_thread(void* pipeline_context) { RECEIVE_THREAD_FAIL(); } if (status == STATUS_MANIFEST) { - if (receive_manifest(file_descriptor, config, &status) != 0) + if (receive_manifest(file_descriptor, config, &status) != 0) { RECEIVE_THREAD_FAIL(); + } } if (status != STATUS_FINISHED) RECEIVE_THREAD_FAIL(); diff --git a/src/shared/protocol.c b/src/shared/protocol.c index 8e90001..f386a26 100644 --- a/src/shared/protocol.c +++ b/src/shared/protocol.c @@ -27,6 +27,13 @@ static once_flag bw_mutex_once = ONCE_FLAG_INIT; static __thread unsigned long long total_allocated_bytes; +void protocol_release_memory(size_t charge) { + if ((unsigned long long)charge >= total_allocated_bytes) + total_allocated_bytes = 0; + else + total_allocated_bytes -= charge; +} + void io_set_fds(int read_fd, int write_fd) { io_read_fd = read_fd; io_write_fd = write_fd; @@ -43,17 +50,20 @@ static void bw_mutex_init(void) { void io_set_bwlimit(unsigned long long bytes_per_sec) { call_once(&bw_mutex_once, bw_mutex_init); mtx_lock(&bw_mutex); - io_bwlimit = bytes_per_sec; + io_bwlimit = + bytes_per_sec > (unsigned long long)LLONG_MAX ? (unsigned long long)LLONG_MAX : bytes_per_sec; bw_tokens = (long long)io_bwlimit; clock_gettime(CLOCK_MONOTONIC, &bw_last_refill); mtx_unlock(&bw_mutex); } static void bw_throttle(size_t bytes_written) { - if (io_bwlimit == 0) - return; call_once(&bw_mutex_once, bw_mutex_init); mtx_lock(&bw_mutex); + if (io_bwlimit == 0) { + mtx_unlock(&bw_mutex); + return; + } struct timespec now; clock_gettime(CLOCK_MONOTONIC, &now); @@ -81,6 +91,15 @@ static void bw_throttle(size_t bytes_written) { mtx_unlock(&bw_mutex); } +static bool bw_enabled(void) { + bool enabled; + call_once(&bw_mutex_once, bw_mutex_init); + mtx_lock(&bw_mutex); + enabled = io_bwlimit != 0; + mtx_unlock(&bw_mutex); + return enabled; +} + void io_set_ssl(SSL* ssl) { io_ssl = ssl; } @@ -105,6 +124,8 @@ static int deadline_remaining_ms(const struct timespec* deadline) { } bool send_n_data(int file_descriptor, const void* data, size_t data_size) { + if (!data && data_size != 0) + return false; log_message(LOG_LEVEL_DEBUG, " Sending n Data: %zu", data_size); int fd = io_fd(io_write_fd, file_descriptor); struct timespec deadline; @@ -114,7 +135,7 @@ bool send_n_data(int file_descriptor, const void* data, size_t data_size) { ssize_t total_bytes_send = 0; while ((size_t)total_bytes_send < data_size) { size_t chunk = data_size - total_bytes_send; - if (io_bwlimit > 0 && chunk > 65536) + if (bw_enabled() && chunk > 65536) chunk = 65536; struct pollfd pfd = {.fd = fd, .events = wait_events}; int poll_result = poll(&pfd, 1, deadline_remaining_ms(&deadline)); @@ -144,6 +165,8 @@ bool send_n_data(int file_descriptor, const void* data, size_t data_size) { } bw_throttle((size_t)bytes_send); total_bytes_send += bytes_send; + if (io_ssl) + wait_events = POLLOUT; } log_message(LOG_LEVEL_DEBUG, " Send n Data: %zu", total_bytes_send); return true; @@ -160,20 +183,22 @@ bool receive_n_data(int file_descriptor, void* data, size_t data_size) { size_t total_bytes_received = 0; short wait_events = POLLIN; while (total_bytes_received < data_size) { - struct pollfd pfd = {.fd = fd, .events = wait_events}; - int poll_result = poll(&pfd, 1, deadline_remaining_ms(&deadline)); - if (poll_result == 0) { - log_message(LOG_LEVEL_ERROR, "Receive timeout after %ds", RECEIVE_TIMEOUT_SEC); - return false; + if (!io_ssl || SSL_pending(io_ssl) == 0) { + struct pollfd pfd = {.fd = fd, .events = wait_events}; + int poll_result = poll(&pfd, 1, deadline_remaining_ms(&deadline)); + if (poll_result == 0) { + log_message(LOG_LEVEL_ERROR, "Receive timeout after %ds", RECEIVE_TIMEOUT_SEC); + return false; + } + if (poll_result < 0) { + if (errno == EINTR) + continue; + return false; + } + /* POLLHUP may accompany the final readable bytes on pipes/sockets. */ + if (pfd.revents & (POLLERR | POLLNVAL)) + return false; } - if (poll_result < 0) { - if (errno == EINTR) - continue; - return false; - } - /* POLLHUP may accompany the final readable bytes on pipes/sockets. */ - if (pfd.revents & (POLLERR | POLLNVAL)) - return false; ssize_t bytes_received; if (io_ssl) @@ -196,7 +221,9 @@ bool receive_n_data(int file_descriptor, void* data, size_t data_size) { log_message(LOG_LEVEL_ERROR, "Could not receive bytes"); return false; } - total_bytes_received += bytes_received; + total_bytes_received += (size_t)bytes_received; + if (io_ssl) + wait_events = POLLIN; } log_message(LOG_LEVEL_DEBUG, " Received n Data: %zu", total_bytes_received); return true; @@ -247,8 +274,7 @@ char* receive_str(int file_descriptor) { size_t size; if (!receive_n_data(file_descriptor, &size, sizeof(size_t))) return NULL; - if (size > MAX_STRING_SIZE || size > SIZE_MAX - 1 || - size + 1 > MAX_CONNECTION_MEMORY - total_allocated_bytes) { + if (size > MAX_STRING_SIZE || size > SIZE_MAX - 1) { log_message(LOG_LEVEL_ERROR, "String size %zu exceeds maximum %llu", size, (unsigned long long)MAX_STRING_SIZE); return NULL; @@ -266,12 +292,13 @@ char* receive_str(int file_descriptor) { return NULL; } data[size] = '\0'; - total_allocated_bytes += size + 1; log_message(LOG_LEVEL_DEBUG, "Received String: %s", data); return data; } bool send_data(int file_descriptor, const Data* data) { + if (!data || (!data->data && data->size != 0)) + return false; unsigned long long data_size = data->size; if (!send_n_data(file_descriptor, &data_size, sizeof(unsigned long long))) return false; @@ -281,11 +308,11 @@ bool send_data(int file_descriptor, const Data* data) { return true; } -Data* receive_data(int file_descriptor) { +Data* receive_data_limited(int file_descriptor, unsigned long long maximum_size) { unsigned long long size = 0; if (!receive_n_data(file_descriptor, &size, sizeof(unsigned long long))) return NULL; - if (size > MAX_DATA_PAYLOAD_SIZE) { + if (size > MAX_DATA_PAYLOAD_SIZE || size > maximum_size) { log_message(LOG_LEVEL_ERROR, "Data size %llu exceeds maximum %llu", size, (unsigned long long)MAX_DATA_PAYLOAD_SIZE); return NULL; @@ -302,16 +329,21 @@ Data* receive_data(int file_descriptor) { free(data); return NULL; } - total_allocated_bytes += allocation_size; log_message(LOG_LEVEL_DEBUG, "Received %lld data", size); Data* result = data_create(data, (size_t)size); if (!result) { free(data); - total_allocated_bytes -= allocation_size; + return NULL; } + total_allocated_bytes += allocation_size; + result->protocol_charge = allocation_size; return result; } +Data* receive_data(int file_descriptor) { + return receive_data_limited(file_descriptor, MAX_DATA_PAYLOAD_SIZE); +} + bool send_int(int file_descriptor, int data) { if (!send_n_data(file_descriptor, &data, sizeof(int))) return false; diff --git a/src/shared/protocol.h b/src/shared/protocol.h index ded825f..ab21b06 100644 --- a/src/shared/protocol.h +++ b/src/shared/protocol.h @@ -10,6 +10,8 @@ /* Maximum allowed data payload size for receive_data (100 MB) */ #define MAX_DATA_PAYLOAD_SIZE (100ULL * 1024 * 1024) +/* Maximum uncompressed file payload accepted by the receiver. */ +#define MAX_RECEIVE_FILE_SIZE (64ULL * 1024 * 1024) /* Maximum chunk size (64 MB) — prevents unbounded allocation from the wire */ #define MAX_CHUNK_SIZE (64ULL * 1024 * 1024) @@ -46,6 +48,7 @@ bool send_str(int file_descriptor, const char* data); char* receive_str(int file_descriptor); bool send_data(int file_descriptor, const Data* data); Data* receive_data(int file_descriptor); +Data* receive_data_limited(int file_descriptor, unsigned long long maximum_size); bool send_int(int file_descriptor, int data); bool receive_int(int file_descriptor, int* data); bool send_status(int file_descriptor, Status status); diff --git a/src/shared/transport_tls.c b/src/shared/transport_tls.c index 15d5727..f280e8d 100644 --- a/src/shared/transport_tls.c +++ b/src/shared/transport_tls.c @@ -105,7 +105,10 @@ static SSL* wrap_fd_with_ssl(int fd, SSL_CTX* ctx, bool is_server, const char* h log_message(LOG_LEVEL_ERROR, "Failed to create SSL object"); return NULL; } - SSL_set_fd(ssl, fd); + if (SSL_set_fd(ssl, fd) != 1) { + SSL_free(ssl); + return NULL; + } // Enable hostname verification for client connections when a hostname is provided. // Must be done before SSL_connect to take effect during the handshake. @@ -154,8 +157,10 @@ struct tls_child_ctx { static void tls_child_fn(int fd, void* arg) { struct tls_child_ctx* ctx = (struct tls_child_ctx*)arg; SSL* ssl = wrap_fd_with_ssl(fd, ctx->ssl_ctx, true, NULL); - if (!ssl) + if (!ssl) { + io_set_ssl(NULL); return; + } io_set_ssl(ssl); ctx->handler(fd); SSL_shutdown(ssl); @@ -184,6 +189,10 @@ bool client_connect_tls(Client* client, char* host, int port, const char* cert_p int err = getaddrinfo(host, port_str, &hints, &result); if (err != 0 || result == NULL) { fprintf(stderr, "Could not resolve host: %s (%s)\n", host, gai_strerror(err)); + if (client->file_descriptor >= 0) { + close(client->file_descriptor); + client->file_descriptor = -1; + } return false; } @@ -216,12 +225,18 @@ bool client_connect_tls(Client* client, char* host, int port, const char* cert_p if (!connected) { perror("Could not connect to Server!"); + if (client->file_descriptor >= 0) + close(client->file_descriptor); + client->file_descriptor = -1; return false; } SSL_CTX* ctx = create_ssl_ctx(false, cert_path, key_path, ca_path); - if (!ctx) + if (!ctx) { + close(client->file_descriptor); + client->file_descriptor = -1; return false; + } client->ssl_ctx = ctx; // Pass the server hostname for TLS hostname verification (SSL_set1_host @@ -231,6 +246,8 @@ bool client_connect_tls(Client* client, char* host, int port, const char* cert_p if (!ssl) { SSL_CTX_free(ctx); client->ssl_ctx = NULL; + close(client->file_descriptor); + client->file_descriptor = -1; return false; } diff --git a/src/shared/utils.c b/src/shared/utils.c index 89c4761..0ba6d8a 100644 --- a/src/shared/utils.c +++ b/src/shared/utils.c @@ -14,14 +14,22 @@ static int authorized_root_fd = -1; static char* authorized_root_path; -void utils_set_authorized_root(int fd, const char* canonical_path) { +bool utils_set_authorized_root(int fd, const char* canonical_path) { + char* path_copy = canonical_path ? str_dup(canonical_path) : NULL; + if (canonical_path && !path_copy) { + authorized_root_fd = -1; + free(authorized_root_path); + authorized_root_path = NULL; + return false; + } authorized_root_fd = fd; free(authorized_root_path); - authorized_root_path = canonical_path ? str_dup(canonical_path) : NULL; + authorized_root_path = path_copy; + return true; } void utils_set_authorized_root_fd(int fd) { - utils_set_authorized_root(fd, NULL); + (void)utils_set_authorized_root(fd, NULL); } static bool path_is_within_root(const char* root, const char* path) { @@ -50,11 +58,15 @@ static int open_authorized_destination(const char* dest_root) { char* saveptr = NULL; char* component = strtok_r(relative, "/", &saveptr); while (component) { - if (strcmp(component, ".") == 0 || strcmp(component, "..") == 0) { + if (strcmp(component, "..") == 0) { free(relative); close(dirfd); return -1; } + if (strcmp(component, ".") == 0) { + component = strtok_r(NULL, "/", &saveptr); + continue; + } int next = openat(dirfd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); if (next < 0) { free(relative); @@ -71,52 +83,41 @@ static int open_authorized_destination(const char* dest_root) { } bool mkdir_r(const char* path) { - size_t path_len = strlen(path); - char* path_duplicate = malloc(path_len + 1); - if (!path_duplicate) + if (!path || *path == '\0') return false; - memcpy(path_duplicate, path, path_len + 1); - size_t capacity = path_len + 2; - char* path_current = (char*)malloc(capacity * sizeof(char)); - if (!path_current) { - free(path_duplicate); + char* duplicate = str_dup(path); + if (!duplicate) + return false; + int dirfd = open(path[0] == '/' ? "/" : ".", O_RDONLY | O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW); + if (dirfd < 0) { + free(duplicate); return false; } - char* path_current_position = path_current; - if (path[0] == '/') { - path_current[0] = '/'; - path_current[1] = '\0'; - path_current_position += 1; - } else { - path_current[0] = '\0'; - } - const char* delimiter = "/"; - char* saveptr; - const char* part = strtok_r(path_duplicate, delimiter, &saveptr); bool ok = true; - while (part != NULL) { - size_t part_len = strlen(part); - if ((size_t)(path_current_position - path_current) + part_len + 2 > capacity) { + char* saveptr = NULL; + char* component = strtok_r(duplicate, "/", &saveptr); + while (component) { + if (strcmp(component, "..") == 0) { ok = false; break; } - memcpy(path_current_position, part, part_len); - path_current_position += part_len; - path_current_position[0] = '/'; - path_current_position[1] = '\0'; - path_current_position++; - struct stat st; - if (stat(path_current, &st) != 0) { - if (mkdir(path_current, 0755) != 0) { - perror("Could not create directory"); + if (strcmp(component, ".") != 0) { + int next = openat(dirfd, component, O_RDONLY | O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW); + if (next < 0 && errno == ENOENT) { + if (mkdirat(dirfd, component, 0755) == 0 || errno == EEXIST) + next = openat(dirfd, component, O_RDONLY | O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW); + } + if (next < 0) { ok = false; break; } + close(dirfd); + dirfd = next; } - part = strtok_r(NULL, delimiter, &saveptr); + component = strtok_r(NULL, "/", &saveptr); } - free(path_duplicate); - free(path_current); + close(dirfd); + free(duplicate); return ok; } @@ -207,15 +208,20 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes close(scanfd); return false; } - bool all_removed = true; bool operation_ok = true; const struct dirent* entry; while ((entry = readdir(dir)) != NULL) { if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) continue; char* child_rel = path_cat((char*)rel_path, entry->d_name); + if (!child_rel) { + operation_ok = false; + continue; + } struct stat st; if (fstatat(dirfd, entry->d_name, &st, AT_SYMLINK_NOFOLLOW) != 0) { + if (errno != ENOENT) + operation_ok = false; free(child_rel); continue; } @@ -229,18 +235,23 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes bool child_removed = false; if (childfd >= 0) { child_removed = delete_extras_fd(childfd, child_rel, manifest, max_delete, deleted_count); + if (!child_removed) + operation_ok = false; close(childfd); + } else if (errno != ENOENT) { + operation_ok = false; } if (child_removed && !is_dir_in_manifest(child_rel, manifest)) { if (*deleted_count >= max_delete) { operation_ok = false; - } else if (unlinkat(dirfd, entry->d_name, AT_REMOVEDIR) != 0 && errno != ENOENT) { - operation_ok = false; } else { - (*deleted_count)++; + if (unlinkat(dirfd, entry->d_name, AT_REMOVEDIR) != 0) { + if (errno != ENOENT) + operation_ok = false; + } else { + (*deleted_count)++; + } } - } else if (!child_removed) { - all_removed = false; } } else { // Check if relative path is in manifest @@ -257,27 +268,32 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes free(child_rel); continue; } - if (unlinkat(dirfd, entry->d_name, 0) != 0 && errno != ENOENT) - operation_ok = false; - else + if (unlinkat(dirfd, entry->d_name, 0) != 0) { + if (errno != ENOENT) + operation_ok = false; + } else { (*deleted_count)++; + } fprintf(stderr, " Deleted: %s\n", child_rel); - } else { - all_removed = false; } } free(child_rel); } closedir(dir); - (void)all_removed; return operation_ok; } bool delete_extras_limited(const char* dest_root, ArrayList* manifest, size_t max_delete) { + if (!manifest) + return false; int rootfd; if (authorized_root_fd >= 0) { - rootfd = - authorized_root_path ? open_authorized_destination(dest_root) : dup(authorized_root_fd); + if (authorized_root_path) + rootfd = open_authorized_destination(dest_root); + else if (dest_root == NULL) + rootfd = dup(authorized_root_fd); + else + rootfd = -1; } else { rootfd = open(dest_root, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); } @@ -296,10 +312,10 @@ bool delete_extras(const char* dest_root, ArrayList* manifest) { bool has_path_traversal(const char* path) { if (!path) - return false; + return true; char* dup = str_dup(path); if (!dup) - return false; + return true; char* saveptr; const char* part = strtok_r(dup, "/", &saveptr); while (part) { @@ -327,6 +343,8 @@ char* path_cat(const char* path1, const char* path2) { offset = 1; path2_len -= 1; } + if (path1_len > SIZE_MAX - path2_len - 2) + return NULL; char* new_path = malloc(path1_len + path2_len + 2); if (new_path == NULL) return NULL; diff --git a/src/shared/utils.h b/src/shared/utils.h index 6d3e8d2..ef1cf80 100644 --- a/src/shared/utils.h +++ b/src/shared/utils.h @@ -11,7 +11,9 @@ char* path_cat(const char* path1, const char* path2); bool glob_match(const char* pattern, const char* str); bool delete_extras(const char* dest_root, ArrayList* manifest); bool delete_extras_limited(const char* dest_root, ArrayList* manifest, size_t max_delete); -void utils_set_authorized_root(int fd, const char* canonical_path); +bool utils_set_authorized_root(int fd, const char* canonical_path); +/* The fd-only compatibility form is fail-closed for path-based operations; + * callers should use utils_set_authorized_root with the canonical identity. */ void utils_set_authorized_root_fd(int fd); bool has_path_traversal(const char* path); diff --git a/tests/test_array_list.c b/tests/test_array_list.c index 1bf05d9..2e8316a 100644 --- a/tests/test_array_list.c +++ b/tests/test_array_list.c @@ -12,11 +12,15 @@ static void test_destroyer(void* item) { void test_array_list() { ArrayList* list = array_list_create(free); EXPECT_NOT_NULL(list); + if (!list) + return; EXPECT_EQ_INT(list->size, 0); EXPECT_EQ_INT(list->capacity, 100); // Test adding int* val1 = malloc(sizeof(int)); + if (!val1) + return; *val1 = 42; array_list_add(list, val1); EXPECT_EQ_INT(list->size, 1); @@ -26,6 +30,8 @@ void test_array_list() { // Initial capacity is 100. Let's add 105 elements. for (int i = 0; i < 105; i++) { int* val = malloc(sizeof(int)); + if (!val) + return; *val = i; array_list_add(list, val); } diff --git a/tests/test_compression.c b/tests/test_compression.c index ade4812..4e0d492 100644 --- a/tests/test_compression.c +++ b/tests/test_compression.c @@ -13,6 +13,8 @@ static void test_data_compress_decompress_roundtrip() { size_t len = strlen(original); char* buf = malloc(len); + if (!buf) + return; memcpy(buf, original, len); Data* original_data = data_create(buf, len); EXPECT_NOT_NULL(original_data); diff --git a/tests/test_property.c b/tests/test_property.c index 4bcf4ef..4a2f45d 100644 --- a/tests/test_property.c +++ b/tests/test_property.c @@ -14,6 +14,8 @@ static Data* random_data(int min_size, int max_size) { int size = min_size + rand() % (max_size - min_size + 1); char* buf = malloc(size); + if (!buf) + return NULL; for (int i = 0; i < size; i++) buf[i] = (char)(rand() % 256); return data_create(buf, size); @@ -23,10 +25,16 @@ static void test_property_compress_roundtrip() { for (int iter = 0; iter < 10; iter++) { Data* original = random_data(1, 10000); EXPECT_NOT_NULL(original); + if (!original) + return; size_t orig_size = original->size; void* orig_copy = malloc(orig_size); EXPECT_NOT_NULL(orig_copy); + if (!orig_copy) { + data_destroy(original); + return; + } memcpy(orig_copy, original->data, orig_size); Data* compressed = data_compress(original, 3); @@ -81,6 +89,8 @@ static void test_property_chunk_roundtrip() { int content_len = 1 + rand() % 4096; char* content = malloc(content_len); + if (!content) + return; for (int i = 0; i < content_len; i++) content[i] = (char)(rand() % 256); diff --git a/tests/test_queue.c b/tests/test_queue.c index 3ab6d3c..c518370 100644 --- a/tests/test_queue.c +++ b/tests/test_queue.c @@ -119,9 +119,13 @@ static void test_queue_destroyer() { destroyer_calls = 0; Queue* q = queue_create(5, my_destroyer); EXPECT_NOT_NULL(q); + if (!q) + return; for (int i = 0; i < 3; i++) { int* val = malloc(sizeof(int)); + if (!val) + break; *val = i; queue_enqueue(q, val); } @@ -181,6 +185,8 @@ static void test_queue_multithreaded() { for (int i = 1; i <= 100; i++) { int* val = malloc(sizeof(int)); + if (!val) + break; *val = i; queue_enqueue_multithreaded(q, val, &mutex, &cnd_empty, &cnd_full); } diff --git a/tests/test_stress.c b/tests/test_stress.c index c73f46e..0df0791 100644 --- a/tests/test_stress.c +++ b/tests/test_stress.c @@ -32,6 +32,8 @@ static int mpmc_producer_func(void* arg) { ProducerCtx* ctx = (ProducerCtx*)arg; for (int i = 1; i <= ITEMS_PER_PRODUCER; i++) { int* val = malloc(sizeof(int)); + if (!val) + return thrd_error; *val = ctx->producer_id * ITEMS_PER_PRODUCER + i; queue_enqueue_multithreaded(ctx->q, val, ctx->mutex, ctx->cnd_empty, ctx->cnd_full); } @@ -121,6 +123,8 @@ static int bp_producer_func(void* arg) { BackpressureCtx* ctx = (BackpressureCtx*)arg; for (int i = 0; i < 5; i++) { int* val = malloc(sizeof(int)); + if (!val) + return thrd_error; *val = i + 1; queue_enqueue_multithreaded(ctx->q, val, ctx->mutex, ctx->cnd_empty, ctx->cnd_full); ctx->items_sent++; @@ -191,9 +195,13 @@ static void test_queue_rapid_create_destroy() { for (int i = 0; i < 100; i++) { Queue* q = queue_create(4, free); EXPECT_NOT_NULL(q); + if (!q) + return; for (int j = 0; j < 3; j++) { int* val = malloc(sizeof(int)); + if (!val) + break; *val = j; queue_enqueue(q, val); }