fix: address remaining PR review findings

src/shared/chunk.c

@@ -215,13 +215,14 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) {
       return NULL;
     }
     memcpy(file_data, data_pointer, file_data_size);
-    data_destroy(file->data);
-    file->data = data_create(file_data, file_data_size);
-    if (file->data == NULL) {
+    Data* replacement = data_create(file_data, file_data_size);
+    if (replacement == NULL) {
       file_destroy(file);
       array_list_delete(files);
       return NULL;
     }
+    data_destroy(file->data);
+    file->data = replacement;
     data_pointer += file_data_size;
     remaining_size -= file_data_size;
 

src/shared/config.c

@@ -121,7 +121,8 @@ static bool validate_received_config(const Config* config) {
          valid_wire_bool(config->delete_after) && valid_wire_bool(config->relative) &&
          valid_wire_bool(config->prune_empty_dirs) && valid_wire_bool(config->partial) &&
          valid_wire_bool(config->delete_before) && valid_wire_bool(config->checksum) &&
-         config->compression_level >= 1 && config->compression_level <= 22 &&
+         (!config->use_compression ||
+          (config->compression_level >= 1 && config->compression_level <= 22)) &&
          config->chunk_size > 0 && config->chunk_size <= MAX_CHUNK_SIZE &&
          config->delta_block_size >= DELTA_BLOCK_SIZE_MIN &&
          config->delta_block_size <= DELTA_BLOCK_SIZE_MAX &&

src/shared/file.c

@@ -23,6 +23,8 @@
 #include "protocol.h"
 #include "utils.h"
 
+#define MAX_SERVER_DELETE_COUNT 100000U
+
 bool file_checksum(File* file, uint64_t* checksum) {
   if (!file || !checksum || !file->data)
     return false;
@@ -158,15 +160,19 @@ static bool to_disk_secure(const char* path, const void* data, unsigned long lon
 static int open_secure_parent(const char* path, char** leaf_out, bool create_dirs);
 
 bool file_path_exists_secure(const char* path) {
-  if (!path)
+  struct stat st;
+  return file_stat_secure(path, &st);
+}
+
+bool file_stat_secure(const char* path, struct stat* st) {
+  if (!path || !st)
     return false;
   char* leaf = NULL;
   int parent_fd = open_secure_parent(path, &leaf, false);
   if (parent_fd < 0)
     return false;
-  struct stat st;
   int fd = openat(parent_fd, leaf, O_RDONLY | O_CLOEXEC | O_NOFOLLOW);
-  bool exists = fd >= 0 && fstat(fd, &st) == 0;
+  bool exists = fd >= 0 && fstat(fd, st) == 0;
   if (fd >= 0)
     close(fd);
   close(parent_fd);
@@ -457,8 +463,16 @@ static File* receive_delta_file(int fd, const Config* config, const char* check_
       }
     }
 
+    Data* replacement = data_create(new_data, (size_t)new_size);
+    if (replacement == NULL) {
+      file_destroy(file);
+      free(old_data);
+      delta_signature_destroy(sig);
+      send_status(fd, STATUS_ERROR);
+      return NULL;
+    }
     data_destroy(file->data);
-    file->data = data_create(new_data, (size_t)new_size);
+    file->data = replacement;
 
     free(old_data);
     delta_signature_destroy(sig);
@@ -510,6 +524,7 @@ static File* receive_delta_file(int fd, const Config* config, const char* check_
 
   delta_signature_destroy(sig);
   free(old_data);
+  send_status(fd, STATUS_ERROR);
   return NULL;
 }
 
@@ -1085,7 +1100,8 @@ int receive_manifest(int fd, const Config* config, int* next_status) {
     return *status_out == STATUS_FINISHED ? 0 : -1;
   }
   fprintf(stderr, "Deleting files not in manifest...\n");
-  bool deletion_ok = delete_extras(config->receive_root_directory, manifest);
+  bool deletion_ok =
+      delete_extras_limited(config->receive_root_directory, manifest, MAX_SERVER_DELETE_COUNT);
   array_list_delete(manifest);
   return deletion_ok ? 0 : -1;
 }

src/shared/file.h

@@ -41,6 +41,7 @@ bool file_save_to_disk(const char* root_directory, const File* file, const Confi
 void 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);
 int receive_manifest(int fd, const Config* config, int* next_status);
 
 #endif

src/shared/metadata.c

@@ -76,6 +76,11 @@ FileMetadata* metadata_from_buf(char** buf) {
   memcpy(&mtime_nsec, *buf, sizeof(mtime_nsec));
   *buf += sizeof(mtime_nsec);
   m->mtime_nsec = (long)mtime_nsec;
+  if (present != 1 || mtime_nsec < 0 || mtime_nsec >= 1000000000LL || mode < 0 || uid < 0 ||
+      gid < 0) {
+    free(m);
+    return NULL;
+  }
   return m;
 }
 
@@ -175,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 & 0777 & ~(S_IWGRP | S_IWOTH);
+  mode_t safe_mode = metadata->mode & 07777 & ~(S_ISUID | S_ISGID);
   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
@@ -193,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 & 0777 & ~(S_IWGRP | S_IWOTH);
+  mode_t safe_mode = metadata->mode & 07777 & ~(S_ISUID | S_ISGID);
   if (fchmod(fd, safe_mode) != 0)
     ok = false;
   /* Client uid/gid values are deliberately not authoritative. */

src/shared/protocol.c

@@ -13,7 +13,6 @@
 
 #define RECEIVE_TIMEOUT_SEC 60 /* 60 second per-message timeout */
 #define SEND_TIMEOUT_SEC 60
-#define MAX_CONNECTION_MEMORY (1024ULL * 1024 * 1024) /* 1 GB total per connection */
 
 static __thread int io_read_fd = -1;
 static __thread int io_write_fd = -1;
@@ -25,15 +24,12 @@ 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 = 0;
-
 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) {
@@ -247,8 +243,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,7 +261,6 @@ 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;
 }
@@ -291,12 +285,6 @@ 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 (%llu + %llu > %llu)",
-                (unsigned long long)total_allocated_bytes, size,
-                (unsigned long long)MAX_CONNECTION_MEMORY);
-    return NULL;
-  }
   void* data = malloc(allocation_size);
   if (data == NULL)
     return NULL;
@@ -304,12 +292,10 @@ 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

@@ -7,6 +7,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <stdint.h>
 #include <sys/stat.h>
 #include <unistd.h>
 
@@ -19,6 +20,10 @@ void utils_set_authorized_root(int fd, const char* canonical_path) {
   authorized_root_path = canonical_path ? str_dup(canonical_path) : NULL;
 }
 
+void utils_set_authorized_root_fd(int fd) {
+  utils_set_authorized_root(fd, 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] == '/');
@@ -192,7 +197,8 @@ static bool is_dir_in_manifest(const char* rel_path, ArrayList* manifest) {
   return false;
 }
 
-static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifest) {
+static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifest,
+                             size_t max_delete, size_t* deleted_count) {
   int scanfd = dup(dirfd);
   if (scanfd < 0)
     return false;
@@ -222,7 +228,7 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes
       int childfd = openat(dirfd, entry->d_name, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
       bool child_removed = false;
       if (childfd >= 0) {
-        child_removed = delete_extras_fd(childfd, child_rel, manifest);
+        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) &&
@@ -241,8 +247,15 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes
         }
       }
       if (!found) {
+        if (*deleted_count >= max_delete) {
+          operation_ok = false;
+          free(child_rel);
+          continue;
+        }
         if (unlinkat(dirfd, entry->d_name, 0) != 0 && errno != ENOENT)
           operation_ok = false;
+        else
+          (*deleted_count)++;
         fprintf(stderr, "  Deleted: %s\n", child_rel);
       } else {
         all_removed = false;
@@ -255,18 +268,23 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes
   return operation_ok;
 }
 
-bool delete_extras(const char* dest_root, ArrayList* manifest) {
+bool delete_extras_limited(const char* dest_root, ArrayList* manifest, size_t max_delete) {
   int rootfd = authorized_root_fd >= 0
                    ? open_authorized_destination(dest_root)
                    : open(dest_root, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
   if (rootfd < 0)
     return false;
-  bool ok = delete_extras_fd(rootfd, "", manifest);
+  size_t deleted_count = 0;
+  bool ok = delete_extras_fd(rootfd, "", manifest, max_delete, &deleted_count);
   if (close(rootfd) != 0)
     ok = false;
   return ok;
 }
 
+bool delete_extras(const char* dest_root, ArrayList* manifest) {
+  return delete_extras_limited(dest_root, manifest, SIZE_MAX);
+}
+
 bool has_path_traversal(const char* path) {
   if (!path)
     return false;

src/shared/utils.h

@@ -2,6 +2,7 @@
 #define UTILS_H
 
 #include "array_list.h"
+#include <stddef.h>
 #include <stdbool.h>
 
 bool mkdir_r(const char* path);
@@ -9,7 +10,9 @@ 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);
+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);
+void utils_set_authorized_root_fd(int fd);
 bool has_path_traversal(const char* path);
 
 #endif