fix: harden remaining review findings

src/server/server.c

@@ -203,7 +203,7 @@ void handler(int file_descriptor) {
     close(file_descriptor);
     return;
   }
-  if (ssl && !tls_client_identity_allowed(ssl)) {
+  if (ssl && required_client_cn && !tls_client_identity_allowed(ssl)) {
     log_message(LOG_LEVEL_ERROR, "Rejected TLS client with unauthorized identity");
     config_delete(config);
     close(file_descriptor);

src/shared/chunk.c

@@ -1,5 +1,6 @@
 #include <stddef.h>
 #include <stdint.h>
+#include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -62,15 +63,31 @@ void chunk_destroy(void* item) {
 }
 
 static unsigned long long per_file_serialize_size(File* file, bool use_metadata) {
-  return sizeof(size_t) + strlen(file->path) +
+  unsigned long long size = sizeof(size_t);
-         (use_metadata ? sizeof(int) + (file->metadata ? FILE_METADATA_WIRE_SIZE : 0) : 0) +
+  size_t path_len = strlen(file->path);
-         sizeof(size_t) + file->data->size;
+  unsigned long long metadata_size =
+      use_metadata ? sizeof(int) + (file->metadata ? FILE_METADATA_WIRE_SIZE : 0) : 0;
+  if ((unsigned long long)path_len > ULLONG_MAX - size)
+    return 0;
+  size += path_len;
+  if (metadata_size > ULLONG_MAX - size)
+    return 0;
+  size += metadata_size;
+  if (sizeof(size_t) > ULLONG_MAX - size)
+    return 0;
+  size += sizeof(size_t);
+  if ((unsigned long long)file->data->size > ULLONG_MAX - size)
+    return 0;
+  return size + file->data->size;
 }
 
 Data* chunk_serialize(Chunk* chunk, bool use_metadata) {
   unsigned long long data_size = 0;
   for (int i = 0; i < chunk->element_count; i++) {
-    data_size += per_file_serialize_size(chunk->items[i], use_metadata);
+    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;
+    data_size += file_size;
   }
   Data* data = data_create_empty(data_size);
   if (data == NULL) {
@@ -174,9 +191,15 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) {
       }
       file->metadata = metadata_from_buf(&data_pointer);
       remaining_size -= sizeof(int);
-      if (file->metadata)
+      if (present_flag == 1) {
+        if (file->metadata == NULL) {
+          file_destroy(file);
+          array_list_delete(files);
+          return NULL;
+        }
         remaining_size -= FILE_METADATA_WIRE_SIZE;
       }
+    }
 
     if (remaining_size < sizeof(size_t)) {
       log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for data size");

src/shared/delta.c

@@ -121,6 +121,13 @@ DeltaSignature* delta_signature_deserialize(const Data* data) {
     return NULL;
   }
 
+  if (sig->block_size == 0 || sig->block_size > DELTA_BLOCK_SIZE_MAX ||
+      sig->file_size > DELTA_MAX_FILE_SIZE || sig->file_size == 0 ||
+      (sig->file_size + sig->block_size - 1) / sig->block_size != sig->block_count) {
+    free(sig);
+    return NULL;
+  }
+
   uint64_t expected = sizeof(uint64_t) + sizeof(uint32_t) + sizeof(uint32_t) +
                       (uint64_t)sig->block_count * (sizeof(uint32_t) + sizeof(uint32_t));
   if (data->size < expected) {

src/shared/file.c

@@ -24,6 +24,7 @@
 #include "utils.h"
 
 #define MAX_SERVER_DELETE_COUNT 100000U
+#define MAX_FILE_DATA_SIZE (64ULL * 1024 * 1024)
 
 bool file_checksum(File* file, uint64_t* checksum) {
   if (!file || !checksum || !file->data)
@@ -171,8 +172,8 @@ bool file_stat_secure(const char* path, struct stat* st) {
   int parent_fd = open_secure_parent(path, &leaf, false);
   if (parent_fd < 0)
     return false;
-  int fd = openat(parent_fd, leaf, O_RDONLY | O_CLOEXEC | O_NOFOLLOW);
+  int fd = openat(parent_fd, leaf, O_RDONLY | O_NONBLOCK | O_CLOEXEC | O_NOFOLLOW);
-  bool exists = fd >= 0 && fstat(fd, st) == 0;
+  bool exists = fd >= 0 && fstat(fd, st) == 0 && S_ISREG(st->st_mode);
   if (fd >= 0)
     close(fd);
   close(parent_fd);
@@ -514,6 +515,12 @@ static File* receive_delta_file(int fd, const Config* config, const char* check_
         send_status(fd, STATUS_ERROR);
         return NULL;
       }
+      if (uncompressed->size > MAX_FILE_DATA_SIZE) {
+        data_destroy(uncompressed);
+        file_destroy(file);
+        send_status(fd, STATUS_ERROR);
+        return NULL;
+      }
       file_data = uncompressed;
     }
 
@@ -675,6 +682,12 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) {
       send_status(fd, STATUS_ERROR);
       return NULL;
     }
+    if (uncompressed->size > MAX_FILE_DATA_SIZE) {
+      data_destroy(uncompressed);
+      file_destroy(file);
+      send_status(fd, STATUS_ERROR);
+      return NULL;
+    }
     file_data = uncompressed;
   }
 
@@ -1043,6 +1056,11 @@ File* file_receive(const Config* config, int file_descriptor) {
       file_destroy(file);
       return NULL;
     }
+    if (file_data_uncompressed->size > MAX_FILE_DATA_SIZE) {
+      data_destroy(file_data_uncompressed);
+      file_destroy(file);
+      return NULL;
+    }
     file_data = file_data_uncompressed;
   }
   data_destroy(file->data);

src/shared/metadata.c

@@ -180,7 +180,7 @@ FileMetadata* metadata_receive(int file_descriptor, int* ok) {
 void file_restore_metadata(const char* path, const FileMetadata* metadata) {
   if (metadata == NULL)
     return;
-  mode_t safe_mode = metadata->mode & 07777 & ~(S_ISUID | S_ISGID);
+  mode_t safe_mode = metadata->mode & 0777 & ~(S_IWGRP | S_IWOTH);
   if (chmod(path, safe_mode) != 0)
     log_message(LOG_LEVEL_WARNING, "Failed to chmod %s: %s", path, strerror(errno));
   /* Never apply client-supplied ownership.  The descriptor API below is the
@@ -198,7 +198,7 @@ bool file_restore_metadata_fd(int fd, const FileMetadata* metadata) {
   if (fd < 0 || metadata == NULL)
     return metadata == NULL;
   bool ok = true;
-  mode_t safe_mode = metadata->mode & 07777 & ~(S_ISUID | S_ISGID);
+  mode_t safe_mode = metadata->mode & 0777 & ~(S_IWGRP | S_IWOTH);
   if (fchmod(fd, safe_mode) != 0)
     ok = false;
   /* Client uid/gid values are deliberately not authoritative. */

src/shared/multiprocessing.c

@@ -234,9 +234,9 @@ int receive_thread(void* pipeline_context) {
         }
         char* full_path = path_cat(config->receive_root_directory, check_path);
         struct stat st;
-        bool has_old = full_path && lstat(full_path, &st) == 0;
+        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;
+                     (long long)st.st_mtime == check_mtime && S_ISREG(st.st_mode);
         if (!send_status(file_descriptor, match ? STATUS_OK : STATUS_NEXT))
           RECEIVE_THREAD_FAIL();
         free(full_path);

src/shared/protocol.c

@@ -13,6 +13,7 @@
 
 #define RECEIVE_TIMEOUT_SEC 60 /* 60 second per-message timeout */
 #define SEND_TIMEOUT_SEC 60
+#define MAX_CONNECTION_MEMORY (256ULL * 1024 * 1024) /* bounded cumulative receive budget */
 
 static __thread int io_read_fd = -1;
 static __thread int io_write_fd = -1;
@@ -24,12 +25,15 @@ static struct timespec bw_last_refill = {0, 0};
 static mtx_t bw_mutex;
 static once_flag bw_mutex_once = ONCE_FLAG_INIT;
 
+static __thread unsigned long long total_allocated_bytes;
+
 void io_set_fds(int read_fd, int write_fd) {
   io_read_fd = read_fd;
   io_write_fd = write_fd;
   /* A descriptor switch starts a new transport; never reuse a TLS object
      belonging to a previous connection or test pipe. */
   io_ssl = NULL;
+  total_allocated_bytes = 0;
 }
 
 static void bw_mutex_init(void) {
@@ -243,7 +247,8 @@ 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) {
+  if (size > MAX_STRING_SIZE || size > SIZE_MAX - 1 ||
+      size + 1 > MAX_CONNECTION_MEMORY - total_allocated_bytes) {
     log_message(LOG_LEVEL_ERROR, "String size %zu exceeds maximum %llu", size,
                 (unsigned long long)MAX_STRING_SIZE);
     return NULL;
@@ -261,6 +266,7 @@ 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;
 }
@@ -285,6 +291,10 @@ Data* receive_data(int file_descriptor) {
     return NULL;
   }
   size_t allocation_size = size == 0 ? 1 : (size_t)size;
+  if (allocation_size > MAX_CONNECTION_MEMORY - total_allocated_bytes) {
+    log_message(LOG_LEVEL_ERROR, "Per-connection memory limit exceeded");
+    return NULL;
+  }
   void* data = malloc(allocation_size);
   if (data == NULL)
     return NULL;
@@ -292,10 +302,12 @@ 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 result;
 }

src/shared/utils.c

@@ -231,9 +231,14 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes
         child_removed = delete_extras_fd(childfd, child_rel, manifest, max_delete, deleted_count);
         close(childfd);
       }
-      if (child_removed && !is_dir_in_manifest(child_rel, manifest) &&
+      if (child_removed && !is_dir_in_manifest(child_rel, manifest)) {
-          unlinkat(dirfd, entry->d_name, AT_REMOVEDIR) != 0 && errno != ENOENT) {
+        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)++;
+        }
       } else if (!child_removed) {
         all_removed = false;
       }
@@ -269,9 +274,13 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes
 }
 
 bool delete_extras_limited(const char* dest_root, ArrayList* manifest, size_t max_delete) {
-  int rootfd = authorized_root_fd >= 0
+  int rootfd;
-                   ? open_authorized_destination(dest_root)
+  if (authorized_root_fd >= 0) {
-                   : open(dest_root, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
+    rootfd =
+        authorized_root_path ? open_authorized_destination(dest_root) : dup(authorized_root_fd);
+  } else {
+    rootfd = open(dest_root, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
+  }
   if (rootfd < 0)
     return false;
   size_t deleted_count = 0;