fix: close remaining PR 208 review findings
CI / lint (pull_request) Failing after 33s
CI / build-and-test (pull_request) Has been skipped
CI / sanitizers (address) (pull_request) Has been skipped
CI / sanitizers (undefined) (pull_request) Has been skipped
CI / fuzz-build (pull_request) Has been skipped
CI / coverage (pull_request) Has been skipped
CI / valgrind (pull_request) Has been skipped

This commit is contained in:
2026-08-10 19:10:02 +02:00
parent d367706689
commit 690bf72d7f
11 changed files with 268 additions and 46 deletions
+57 -4
View File
@@ -37,6 +37,8 @@ bool file_checksum(File* file, uint64_t* checksum) {
}
File* file_create(const char* path) {
if (!path)
return NULL;
File* file = (File*)malloc(sizeof(File));
if (file == NULL) {
perror("ERROR: Could not allocate memory for file struct");
@@ -102,6 +104,8 @@ bool file_load_data(File* file) {
if (file == NULL)
return false;
if (file->data->data == NULL) {
if (file->data->size == 0)
return true;
file->data->data = malloc(file->data->size);
if (file->data->data == NULL) {
perror("Could not allocate memory for file data");
@@ -121,6 +125,8 @@ 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)
return false;
const Data* data_to_send = file->data;
Data* compressed_data = NULL;
if (compression_level > 0 && !compression_should_skip(file->path)) {
@@ -151,6 +157,14 @@ static bool to_disk_secure(const char* path, const void* data, unsigned long lon
bool inplace, bool sparse, const FileMetadata* metadata);
static int open_secure_parent(const char* path, char** leaf_out);
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 n = strlen(root);
@@ -648,8 +662,25 @@ static int open_secure_parent(const char* path, char** leaf_out) {
free(copy);
return -1;
}
int fd = (parent[0] == '/') ? open("/", O_RDONLY | O_DIRECTORY | O_CLOEXEC)
: open(".", O_RDONLY | O_DIRECTORY | O_CLOEXEC);
int fd;
if (authorized_root_fd >= 0 && authorized_root_path && path[0] == '/' &&
path_is_within_root(authorized_root_path, path)) {
fd = dup(authorized_root_fd);
size_t root_len = strlen(authorized_root_path);
char* relative = str_dup(path + root_len);
if (!relative) {
free(copy);
free(leaf);
close(fd);
return -1;
}
free(copy);
copy = relative;
parent = dirname(copy);
} else {
fd = (parent[0] == '/') ? open("/", O_RDONLY | O_DIRECTORY | O_CLOEXEC)
: open(".", O_RDONLY | O_DIRECTORY | O_CLOEXEC);
}
if (fd < 0) {
free(copy);
free(leaf);
@@ -861,6 +892,8 @@ done:
bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int compression_level,
bool send_path) {
if (!file || !file->path || !file->data)
return false;
if (compression_level > 0)
return file_send_single_calls(file, file_descriptor, use_metadata, compression_level,
send_path);
@@ -877,6 +910,12 @@ bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int
}
unsigned long long file_size = file->data->size;
struct stat source_stat;
if (fstat(fd, &source_stat) != 0 || !S_ISREG(source_stat.st_mode) ||
(unsigned long long)source_stat.st_size < file_size) {
close(fd);
return false;
}
if (!send_n_data(file_descriptor, &file_size, sizeof(unsigned long long))) {
close(fd);
return false;
@@ -885,8 +924,18 @@ bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int
/* sendfile cannot encrypt TLS records. Keep the framing identical but
route encrypted transfers through the deadline-aware IO layer. */
if (io_get_ssl() != NULL) {
bool loaded = file->data->data != NULL || file_load_data(file);
bool ok = loaded && send_n_data(file_descriptor, file->data->data, (size_t)file_size);
unsigned char buffer[64 * 1024];
unsigned long long remaining = file_size;
bool ok = true;
while (remaining > 0) {
size_t want = remaining > sizeof(buffer) ? sizeof(buffer) : (size_t)remaining;
ssize_t got = read(fd, buffer, want);
if (got <= 0 || !send_n_data(file_descriptor, buffer, (size_t)got)) {
ok = false;
break;
}
remaining -= (unsigned long long)got;
}
close(fd);
return ok;
}
@@ -919,6 +968,10 @@ bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int
close(fd);
return false;
}
if (sent == 0) {
close(fd);
return false;
}
}
close(fd);
+1
View File
@@ -38,6 +38,7 @@ 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);
File* receive_incremental_check(int fd, const Config* config, bool* skipped);
int receive_manifest(int fd, const Config* config, int* next_status);
+2 -2
View File
@@ -177,8 +177,8 @@ void file_restore_metadata(const char* path, FileMetadata* metadata) {
return;
if (chmod(path, metadata->mode & 07777 & ~(S_ISUID | S_ISGID)) != 0)
log_message(LOG_LEVEL_WARNING, "Failed to chmod %s: %s", path, strerror(errno));
if (chown(path, metadata->uid, metadata->gid) != 0)
log_message(LOG_LEVEL_WARNING, "Failed to chown %s: %s", path, strerror(errno));
/* Never apply client-supplied ownership. The descriptor API below is the
receiver write path; retain this legacy API only for compatibility. */
struct timespec times[2];
times[0].tv_sec = 0;
times[0].tv_nsec = UTIME_OMIT;
+11
View File
@@ -1,4 +1,5 @@
#include "multiprocessing.h"
#include "array_list.h"
#include "chunk.h"
#include "config.h"
@@ -13,6 +14,10 @@
#include <string.h>
#include <threads.h>
static bool valid_batch_path(const char* path) {
return path && path[0] != '\0' && path[0] != '/' && !has_path_traversal(path);
}
PipelineContextSender* pipeline_context_sender_create(Config* config, Queue* queue_scanner,
Queue* queue_loader) {
PipelineContextSender* context = malloc(sizeof(PipelineContextSender));
@@ -221,6 +226,12 @@ int receive_thread(void* pipeline_context) {
free(check_path);
RECEIVE_THREAD_FAIL();
}
if (!valid_batch_path(check_path)) {
free(check_path);
if (!send_status(file_descriptor, STATUS_ERROR))
RECEIVE_THREAD_FAIL();
RECEIVE_THREAD_FAIL();
}
char* full_path = path_cat(config->receive_root_directory, check_path);
struct stat st;
bool has_old = full_path && lstat(full_path, &st) == 0;
+9 -1
View File
@@ -10,6 +10,12 @@
#include <sys/stat.h>
#include <unistd.h>
static int authorized_root_fd = -1;
void utils_set_authorized_root_fd(int fd) {
authorized_root_fd = fd;
}
bool mkdir_r(const char* path) {
size_t path_len = strlen(path);
char* path_duplicate = malloc(path_len + 1);
@@ -201,7 +207,9 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes
}
bool delete_extras(const char* dest_root, ArrayList* manifest) {
int rootfd = open(dest_root, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
int rootfd = authorized_root_fd >= 0
? dup(authorized_root_fd)
: open(dest_root, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
if (rootfd < 0)
return false;
bool ok = delete_extras_fd(rootfd, "", manifest);
+1
View File
@@ -9,6 +9,7 @@ char* str_dup(const char* string);
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);
void utils_set_authorized_root_fd(int fd);
bool has_path_traversal(const char* path);
#endif