fix(audit): security, correctness, refactors, docs (no wire change) #306

Merged
TapTap merged 44 commits from fix/audit-cycle into dev 2026-09-21 22:23:33 +02:00
7 changed files with 69 additions and 9 deletions
Showing only changes of commit 5baf120243 - Show all commits
+3 -3
View File
@@ -9,7 +9,7 @@
ArrayList* array_list_create(void (*item_destroyer)(void* item)) {
ArrayList* list = (ArrayList*)protocol_alloc(sizeof(ArrayList));
if (list == NULL) {
log_perror("ERROR: Could not allocate memory for array list struct");
log_message(LOG_LEVEL_ERROR, "%s", "ERROR: Could not allocate memory for array list struct");
return NULL;
}
@@ -47,7 +47,7 @@ static bool array_list_extend(ArrayList* array_list) {
new_capacity = INITIAL_ARRAY_SIZE;
void* new_items = protocol_realloc(array_list->items, new_capacity * sizeof(void*));
if (new_items == NULL) {
log_perror("ERROR: Could not reallocate memory for array list items");
log_message(LOG_LEVEL_ERROR, "%s", "ERROR: Could not reallocate memory for array list items");
return false;
}
array_list->items = new_items;
@@ -73,7 +73,7 @@ void** array_list_to_array(const ArrayList* array_list) {
}
void** array = protocol_alloc(array_list->size * sizeof(void*));
if (array == NULL) {
log_perror("Could not malloc space for array from array list!");
log_message(LOG_LEVEL_ERROR, "%s", "Could not malloc space for array from array list!");
return NULL;
}
memcpy(array, array_list->items, array_list->size * sizeof(void*));
+2 -1
View File
@@ -988,10 +988,11 @@ static bool apply_deferred_path(DeletePlanSession* session, const Config* config
return false;
char* leaf = NULL;
int parent_fd = file_open_secure_parent(full, &leaf, false);
int open_errno = errno;
free(full);
if (parent_fd < 0) {
free(leaf);
return errno == ENOENT || errno == ENOTDIR;
return open_errno == ENOENT || open_errno == ENOTDIR;
}
struct stat st;
if (fstatat(parent_fd, leaf, &st, AT_SYMLINK_NOFOLLOW) != 0) {
+3 -1
View File
@@ -86,11 +86,13 @@ PipelineContextSender* pipeline_context_sender_create(Config* config, Queue* que
return context;
fail:
log_perror("Error initializing synchronization objects");
log_message(LOG_LEVEL_ERROR, "%s", "Error initializing synchronization objects");
if (context->dir_entries_mutex_init)
mtx_destroy(&context->dir_entries_mutex);
if (context->dir_entries)
array_list_delete(context->dir_entries);
if (init >= 7)
mtx_destroy(&context->mutex_progress);
if (init >= 6)
cnd_destroy(&context->condition_not_empty_loader);
if (init >= 5)
+23 -2
View File
@@ -564,6 +564,15 @@ static const char* status_to_string(Status status) {
}
}
/* Reject a raw wire status outside the known enum range before it is handed to
* callers, so an unknown/corrupt frame fails as a protocol error instead of
* being silently interpreted as an unexpected-but-valid verdict. STATUS_OK is
* the first enumerator and STATUS_STATS the last, so the range check accepts
* every status the protocol defines. */
static bool status_is_valid(Status status) {
return status >= STATUS_OK && status <= STATUS_STATS;
}
/* Shared string send/receive implementation. `redact` selects whether the
* payload body is written to the LOG_DEBUG_PROTO debug log: daemon auth material
* (the username and the proof/signature fields) sets it so a --verbose log never
@@ -647,7 +656,7 @@ bool protocol_send_data(ProtocolSession* session, const Data* data) {
return false;
if (!protocol_send_n_data(session, data->data, data_size))
return false;
log_debug_message(LOG_DEBUG_PROTO, "Send %lld data", data_size);
log_debug_message(LOG_DEBUG_PROTO, "Send %llu data", data_size);
return true;
}
@@ -681,7 +690,7 @@ Data* protocol_receive_data_limited(ProtocolSession* session, unsigned long long
protocol_release_memory_for_session(session, allocation_size);
return NULL;
}
log_debug_message(LOG_DEBUG_PROTO, "Received %lld data", size);
log_debug_message(LOG_DEBUG_PROTO, "Received %llu data", size);
Data* result = data_create(data, (size_t)size);
if (!result) {
protocol_release_memory_for_session(session, allocation_size);
@@ -791,6 +800,10 @@ bool protocol_receive_status(ProtocolSession* session, Status* status) {
}
if (!protocol_receive_n_data_until(session, status, sizeof(Status), deadline_ptr))
return false;
if (!status_is_valid(*status)) {
log_message(LOG_LEVEL_ERROR, "Received unknown protocol status %d", *status);
return false;
}
if (!protocol_capture_error_detail(session, status, deadline_ptr, NULL))
return false;
log_debug_message(LOG_DEBUG_PROTO, "Received Status: %s", status_to_string(*status));
@@ -812,6 +825,10 @@ bool protocol_receive_status_timed(ProtocolSession* session, Status* status, int
deadline.tv_sec += timeout_sec;
if (!protocol_receive_n_data_until(session, status, sizeof(Status), &deadline))
return false;
if (!status_is_valid(*status)) {
log_message(LOG_LEVEL_ERROR, "Received unknown protocol status %d", *status);
return false;
}
if (!protocol_capture_error_detail(session, status, &deadline, NULL))
return false;
log_debug_message(LOG_DEBUG_PROTO, "Received Status: %s", status_to_string(*status));
@@ -925,6 +942,10 @@ bool protocol_receive_status_keepalive(ProtocolSession* session, Status* status,
Status received;
if (!protocol_read_status_until(session, &received, &deadline))
return false;
if (!status_is_valid(received)) {
log_message(LOG_LEVEL_ERROR, "Received unknown protocol status %d", received);
return false;
}
if (!protocol_capture_error_detail(session, &received, &deadline, abort_check))
return false;
if (received == STATUS_KEEPALIVE) {
+3
View File
@@ -269,6 +269,9 @@ bool protocol_send_int(ProtocolSession* session, int data);
bool protocol_receive_int(ProtocolSession* session, int* data);
bool protocol_send_status(ProtocolSession* session, Status status);
bool protocol_receive_status(ProtocolSession* session, Status* status);
/* As protocol_receive_status, but with an explicit per-message deadline
* (seconds) instead of the session's configured io_timeout_sec. */
bool protocol_receive_status_timed(ProtocolSession* session, Status* status, int timeout_sec);
bool send_n_data(int file_descriptor, const void* data, size_t data_size);
bool receive_n_data(int file_descriptor, void* data, size_t data_size);
+2 -2
View File
@@ -128,7 +128,7 @@ bool queue_enqueue_multithreaded_cancel(Queue* queue, void* item, mtx_t* mutex,
void* queue_dequeue(Queue* queue) {
if (queue == NULL || queue_is_empty(queue)) {
log_perror("ERROR: Could not dequeue from null or empty queue.");
log_message(LOG_LEVEL_ERROR, "%s", "ERROR: Could not dequeue from null or empty queue.");
return NULL;
}
@@ -145,7 +145,7 @@ bool queue_push(Queue* queue, void* item) {
void* queue_pop(Queue* queue) {
if (queue == NULL || queue_is_empty(queue)) {
log_perror("ERROR: Could not pop from null or empty queue.");
log_message(LOG_LEVEL_ERROR, "%s", "ERROR: Could not pop from null or empty queue.");
return NULL;
}
+33
View File
@@ -216,6 +216,38 @@ static void test_send_receive_status() {
close(p[1]);
}
/* An unknown wire status outside the enum range must be rejected as a protocol
* error instead of being handed to the caller as an unexpected verdict. The
* last known enumerator (STATUS_STATS) must still be accepted, proving the
* validation does not reject legitimate statuses. */
static void test_receive_status_rejects_unknown() {
int p[2];
EXPECT_EQ_INT(pipe(p), 0);
ProtocolSession session;
protocol_session_init(&session, p[0], p[1]);
Status bogus = (Status)(STATUS_STATS + 1);
EXPECT_EQ_INT((int)write(p[1], &bogus, sizeof(bogus)), (int)sizeof(bogus));
Status received = STATUS_OK;
EXPECT_FALSE(protocol_receive_status(&session, &received));
Status negative = (Status)-1;
EXPECT_EQ_INT((int)write(p[1], &negative, sizeof(negative)), (int)sizeof(negative));
EXPECT_FALSE(protocol_receive_status(&session, &received));
Status top = STATUS_STATS;
EXPECT_EQ_INT((int)write(p[1], &top, sizeof(top)), (int)sizeof(top));
EXPECT_TRUE(protocol_receive_status(&session, &received));
EXPECT_EQ_INT((int)received, (int)STATUS_STATS);
Status timed_bogus = (Status)(STATUS_STATS + 7);
EXPECT_EQ_INT((int)write(p[1], &timed_bogus, sizeof(timed_bogus)), (int)sizeof(timed_bogus));
EXPECT_FALSE(protocol_receive_status_timed(&session, &received, 5));
close(p[0]);
close(p[1]);
}
static void test_receive_n_data_truncated() {
int p[2];
EXPECT_EQ_INT(pipe(p), 0);
@@ -723,6 +755,7 @@ void test_protocol() {
test_send_receive_data();
test_send_receive_int();
test_send_receive_status();
test_receive_status_rejects_unknown();
test_protocol_session_io_timeout();
test_protocol_server_io_timeout_floor();
test_send_receive_status_timed();