From 4534284fe917d3f626a0ed0cc0838ca089e1e893 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sat, 8 Aug 2026 20:35:29 +0200 Subject: [PATCH] fix: complete triage security and transfer remediation --- .gitea/workflows/ci.yaml | 9 ++- src/client/client_cli.c | 6 ++ src/server/server.c | 71 ++++++++++++++++++++++- src/shared/file.c | 112 +++++++++++++++++++++++++++++++++++++ src/shared/queue.h | 54 +++++++++--------- src/shared/transport_tcp.c | 3 + src/shared/transport_tls.c | 2 +- tests/test_file.c | 23 ++++++++ 8 files changed, 248 insertions(+), 32 deletions(-) diff --git a/.gitea/workflows/ci.yaml b/.gitea/workflows/ci.yaml index 82cd6af..00c64c0 100644 --- a/.gitea/workflows/ci.yaml +++ b/.gitea/workflows/ci.yaml @@ -2,7 +2,7 @@ name: CI on: push: - branches: [main] + branches: [main, dev] pull_request: jobs: @@ -73,6 +73,13 @@ jobs: - name: Build fuzz targets run: cmake --build build-fuzz -j$(nproc) + - name: Smoke fuzz targets + run: | + for target in build-fuzz/fuzz_*; do + [ -x "$target" ] || continue + timeout 10s "$target" -runs=100 -max_total_time=5 + done + coverage: runs-on: ubuntu-latest container: gitea.tap-tap.win/taptap/fastsync-ci:v9 diff --git a/src/client/client_cli.c b/src/client/client_cli.c index a144c25..2b63c47 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -525,6 +525,12 @@ static bool validate_config(const Config* config) { fprintf(stderr, "Error: --delta cannot be combined with -f (sendfile)\n"); return false; } + if (config->append || config->append_verify) { + fprintf( + stderr, + "Error: --append and --append-verify are not supported yet; refusing to ignore option\n"); + return false; + } if (config->use_tls) { if (!config->tls_cert || !config->tls_key) { fprintf(stderr, "Error: --tls requires --cert and --key\n"); diff --git a/src/server/server.c b/src/server/server.c index 704537b..416b49e 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -15,6 +15,25 @@ #include #include #include +#include +#include +#include + +static char* authorized_root; +static bool allow_delete; + +static bool path_is_within(const char* root, const char* path) { + size_t n = strlen(root); + return strncmp(root, path, n) == 0 && (path[n] == '\0' || path[n] == '/'); +} + +static bool __attribute__((unused)) configure_authorization(const char* root) { + char resolved[PATH_MAX]; + if (!root || !realpath(root, resolved)) + return false; + authorized_root = str_dup(resolved); + return authorized_root != NULL; +} int receive_files(Config* config, int fd) { Status status; @@ -119,6 +138,36 @@ void handler(int file_descriptor) { close(file_descriptor); return; } + if (!authorized_root) { + log_message(LOG_LEVEL_ERROR, "No server-side destination root configured"); + config_delete(config); + 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)) { + log_message(LOG_LEVEL_ERROR, "Rejected destination outside authorized root"); + free(canonical_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 (!config->receive_root_directory) { + config_delete(config); + close(file_descriptor); + return; + } + config->use_delete = config->use_delete && allow_delete; if (config->use_multithreading) { Queue* q = queue_create(100, file_destroy); if (q == NULL) { @@ -178,6 +227,8 @@ static void print_server_usage(void) { printf(" --cert TLS certificate file (PEM)\n"); printf(" --key TLS private key file (PEM)\n"); printf(" --ca TLS CA certificate file (PEM)\n"); + printf(" --destination-root Authorized destination root (default: .)\n"); + printf(" --allow-delete Permit manifest deletion\n"); printf(" -v, --verbose Enable debug logging\n"); printf(" --help Show this help\n"); } @@ -188,6 +239,8 @@ int main(int argc, char* argv[]) { char* tls_key = NULL; char* tls_ca = NULL; int port = 8080; + const char* destination_root = "."; + bool stdio_mode = false; signal(SIGPIPE, SIG_IGN); for (int i = 1; i < argc; i++) { @@ -195,9 +248,7 @@ int main(int argc, char* argv[]) { print_server_usage(); return 0; } else if (strcmp(argv[i], "--stdio") == 0) { - io_set_fds(STDIN_FILENO, STDOUT_FILENO); - handler(STDIN_FILENO); - return 0; + stdio_mode = true; } else if (strcmp(argv[i], "-v") == 0 || strcmp(argv[i], "--verbose") == 0) { set_log_level(LOG_LEVEL_DEBUG); } else if (strcmp(argv[i], "--tls") == 0) { @@ -208,6 +259,10 @@ int main(int argc, char* argv[]) { tls_key = argv[++i]; } else if (strcmp(argv[i], "--ca") == 0 && i + 1 < argc) { tls_ca = argv[++i]; + } else if (strcmp(argv[i], "--destination-root") == 0 && i + 1 < argc) { + destination_root = argv[++i]; + } else if (strcmp(argv[i], "--allow-delete") == 0) { + allow_delete = true; } else if (strcmp(argv[i], "-p") == 0 && i + 1 < argc) { char* end; long p = strtol(argv[++i], &end, 10); @@ -229,6 +284,16 @@ int main(int argc, char* argv[]) { signal(SIGINT, cleanup); signal(SIGTERM, cleanup); + if (!configure_authorization(destination_root)) { + fprintf(stderr, "Error: invalid destination root '%s'\n", destination_root); + return 1; + } + if (stdio_mode) { + io_set_fds(STDIN_FILENO, STDOUT_FILENO); + handler(STDIN_FILENO); + free(authorized_root); + return 0; + } g_server = server_create(port); if (g_server == NULL) { log_message(LOG_LEVEL_ERROR, "Failed to create server"); diff --git a/src/shared/file.c b/src/shared/file.c index e6c14e0..f30adca 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -175,6 +176,17 @@ bool file_save_to_disk(const char* root_directory, File* file, const Config* con 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 && + destination_stat.st_mtime > file->metadata->mtime_sec) { + free(resolved_root); + free(disk_path); + return true; + } + } + if (backup_enabled) { struct stat backup_stat; if (stat(disk_path, &backup_stat) == 0) { @@ -546,8 +558,108 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { return file; } +static int open_secure_parent(const char* path, char** leaf_out) { + char* copy = str_dup(path); + if (!copy) + return -1; + char* parent = dirname(copy); + const char* slash = strrchr(path, '/'); + char* leaf = str_dup(slash ? slash + 1 : path); + if (!leaf) { + free(copy); + return -1; + } + int fd = (parent[0] == '/') ? open("/", O_RDONLY | O_DIRECTORY | O_CLOEXEC) + : open(".", O_RDONLY | O_DIRECTORY | O_CLOEXEC); + if (fd < 0) { + free(copy); + free(leaf); + return -1; + } + char* save = NULL; + char* component = strtok_r(parent, "/", &save); + while (component) { + if (strcmp(component, ".") != 0 && strcmp(component, "..") != 0) { + int next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + if (next < 0 && errno == ENOENT && mkdirat(fd, component, 0755) == 0) + next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + if (next < 0) { + close(fd); + free(copy); + free(leaf); + return -1; + } + close(fd); + fd = next; + } + component = strtok_r(NULL, "/", &save); + } + free(copy); + *leaf_out = leaf; + return fd; +} + +static bool write_all(int fd, const void* data, unsigned long long size) { + const unsigned char* p = data; + unsigned long long done = 0; + while (done < size) { + ssize_t n = write(fd, p + done, (size_t)(size - done)); + if (n < 0 && errno == EINTR) + continue; + if (n <= 0) + return false; + done += (unsigned long long)n; + } + return true; +} + +static bool to_disk_secure(const char* path, const void* data, unsigned long long data_size, + bool inplace, bool sparse) { + char* leaf = NULL; + int dirfd = open_secure_parent(path, &leaf); + if (dirfd < 0) + return false; + int fd = -1; + bool ok = false; + if (inplace) { + fd = openat(dirfd, leaf, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW, 0644); + if (fd >= 0) { + if (!sparse || data_size == 0 || ftruncate(fd, (off_t)data_size) == 0) + ok = write_all(fd, data, data_size); + } + } else { + char tmp[NAME_MAX]; + for (unsigned int i = 0; i < 100 && !ok; ++i) { + snprintf(tmp, sizeof(tmp), ".%s.tmp.%ld.%u", leaf, (long)getpid(), i); + fd = openat(dirfd, tmp, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC | O_NOFOLLOW, 0600); + if (fd < 0) + continue; + if (sparse && data_size > 0) + ok = ftruncate(fd, (off_t)data_size) == 0; + if (ok || (!sparse || data_size == 0)) + ok = write_all(fd, data, data_size); + if (close(fd) != 0) + ok = false; + fd = -1; + if (ok && renameat(dirfd, tmp, dirfd, leaf) != 0) + ok = false; + if (!ok) + unlinkat(dirfd, tmp, 0); + } + } + if (fd >= 0) + close(fd); + close(dirfd); + free(leaf); + return ok; +} + bool to_disk(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse) { + if (!path || (!data && data_size != 0) || has_path_traversal(path)) + return false; + return to_disk_secure(path, data, data_size, inplace, sparse); + /* Kept below only as historical context; all writes use descriptor-relative operations. */ char* tmp_path = NULL; char* directory = NULL; diff --git a/src/shared/queue.h b/src/shared/queue.h index 3d03039..c72659d 100644 --- a/src/shared/queue.h +++ b/src/shared/queue.h @@ -1,30 +1,30 @@ -#ifndef QUEUE_H -#define QUEUE_H - -#include -#include - -typedef struct Queue { - void** items; - int front; - int rear; - int size; - int capacity; - void (*item_destroyer)(void* item); -} Queue; - -Queue* queue_create(int capacity, void (*destroyer)(void* item)); -void queue_destroy(Queue* queue); -bool queue_is_empty(const Queue* queue); -bool queue_is_full(const Queue* queue); -bool queue_enqueue(Queue* queue, void* item); -bool queue_enqueue_multithreaded(Queue* queue, void* item, mtx_t* mutex, cnd_t* condition_not_empty, - cnd_t* condition_not_full); +#ifndef QUEUE_H +#define QUEUE_H + +#include +#include + +typedef struct Queue { + void** items; + int front; + int rear; + int size; + int capacity; + void (*item_destroyer)(void* item); +} Queue; + +Queue* queue_create(int capacity, void (*destroyer)(void* item)); +void queue_destroy(Queue* queue); +bool queue_is_empty(const Queue* queue); +bool queue_is_full(const Queue* queue); +bool queue_enqueue(Queue* queue, void* item); +bool queue_enqueue_multithreaded(Queue* queue, void* item, mtx_t* mutex, cnd_t* condition_not_empty, + cnd_t* condition_not_full); bool queue_enqueue_multithreaded_cancel(Queue* queue, void* item, mtx_t* mutex, cnd_t* condition_not_empty, cnd_t* condition_not_full, const bool* cancelled); -void* queue_dequeue(Queue* queue); -void* queue_dequeue_multithreaded(Queue* queue, mtx_t* mutex, cnd_t* condition_not_empty, - cnd_t* condition_not_full, const bool* other_thread_done); - -#endif +void* queue_dequeue(Queue* queue); +void* queue_dequeue_multithreaded(Queue* queue, mtx_t* mutex, cnd_t* condition_not_empty, + cnd_t* condition_not_full, const bool* other_thread_done); + +#endif diff --git a/src/shared/transport_tcp.c b/src/shared/transport_tcp.c index 93b5aec..49c2fb9 100644 --- a/src/shared/transport_tcp.c +++ b/src/shared/transport_tcp.c @@ -15,6 +15,8 @@ static volatile sig_atomic_t g_active_connections = 0; +static void tcp_apply_socket_timeout(int fd); + static void sigchld_handler(int sig) { (void)sig; int saved_errno = errno; @@ -93,6 +95,7 @@ static void accept_loop(Server* server, void (*child_fn)(int, void*), void* chil perror("Could not accept the connection"); continue; } + tcp_apply_socket_timeout(fd); if ((unsigned int)g_active_connections >= server->max_connections) { log_message(LOG_LEVEL_WARNING, "Max connections (%u) reached, rejecting", server->max_connections); diff --git a/src/shared/transport_tls.c b/src/shared/transport_tls.c index 704b6ff..49fc554 100644 --- a/src/shared/transport_tls.c +++ b/src/shared/transport_tls.c @@ -71,7 +71,7 @@ static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key SSL_CTX_free(ctx); return NULL; } - SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, NULL); + SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, NULL); SSL_CTX_set_verify_depth(ctx, 4); } else { SSL_CTX_set_verify(ctx, SSL_VERIFY_NONE, NULL); diff --git a/tests/test_file.c b/tests/test_file.c index c9f4766..f8a3337 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -126,6 +126,28 @@ static void test_to_disk_creates_dirs() { rmdir("test_nested_tmp"); } +static void test_to_disk_does_not_follow_symlink() { + const char* outside = "test_to_disk_outside.txt"; + const char* link = "test_to_disk_link.txt"; + const char* content = "confined"; + unlink(outside); + unlink(link); + EXPECT_TRUE(to_disk(outside, "outside", 7, false, false)); + EXPECT_EQ_INT(symlink(outside, link), 0); + EXPECT_TRUE(to_disk(link, content, strlen(content), false, false)); + FILE* fp = fopen(outside, "rb"); + char buf[16] = {0}; + EXPECT_NOT_NULL(fp); + if (fp) { + size_t read_count = fread(buf, 1, sizeof(buf) - 1, fp); + EXPECT_TRUE(read_count <= sizeof(buf) - 1); + fclose(fp); + } + EXPECT_EQ_STR(buf, "outside"); + unlink(outside); + unlink(link); +} + static void test_file_content_to_buffer() { const char* content = "Buffer content test"; EXPECT_TRUE(to_disk("test_buffer_file.txt", content, strlen(content), false, false)); @@ -434,6 +456,7 @@ void test_file() { test_file_save_to_disk(); test_to_disk_basic(); test_to_disk_creates_dirs(); + test_to_disk_does_not_follow_symlink(); test_file_content_to_buffer(); test_file_save_to_disk_path_traversal(); test_file_save_to_disk_deep_traversal();