fix: address delegated review findings

README.md

@@ -188,7 +188,7 @@ Config negotiation is sender-driven: the client serializes transfer options and
 7. **Metadata restoration** — `chmod()`, `chown()`, `utimensat()` on the receiving side
 8. **`--delete`** — sender tracks all sent paths; receiver walks destination tree and removes unlisted files/directories
 9. **SSH transport** — `socketpair()` + `fork()` + `execvp("ssh", ...)` with `ControlMaster` and port support
-10. **TLS transport** — OpenSSL `SSL_CTX` with TLS 1.2 minimum, optional CA verification, transparent `SSL_read`/`SSL_write` via `io_set_ssl()`
+10. **TLS transport** — OpenSSL `SSL_CTX` with TLS 1.2 minimum, mutual CA verification, transparent `SSL_read`/`SSL_write` via `io_set_ssl()`
 11. **Path traversal protection** — `has_path_traversal()` rejects any file path containing `..` components, preventing directory escape attacks
 12. **Connection limiting** — server tracks active connections and rejects new ones beyond `max_connections` (default 100)
 13. **Keep-alive** — idle connections receive periodic `STATUS_KEEPALIVE` to detect half-open TCP connections
@@ -202,7 +202,7 @@ Config negotiation is sender-driven: the client serializes transfer options and
 All received file paths are validated by `has_path_traversal()` before any disk operation. Any path containing `..` components is rejected with `STATUS_ERROR`, preventing directory escape attacks.
 
 ### TLS Certificate Verification
-When `--ca` is provided, the server performs mutual TLS verification (`SSL_VERIFY_PEER` with depth 4). Without `--ca`, TLS is still encrypted but peer certificates are not verified.
+TLS requires `--ca` and performs mutual TLS verification (`SSL_VERIFY_PEER` with depth 4). Connections without certificate verification are rejected.
 
 ### Connection Limits
 The server enforces a maximum of 100 concurrent connections (configurable via `max_connections` in `Server`). When the limit is reached, new connections are immediately rejected and closed.
@@ -249,7 +249,7 @@ cmake -B build -S . && cmake --build build -j$(nproc)
 
 ### Server with TLS
 ```bash
-./build/server --tls --cert server.pem --key server-key.pem
+./build/server --tls --cert server.pem --key server-key.pem --ca ca.pem
 ```
 
 ### Server via SSH

src/server/server.c

@@ -58,7 +58,7 @@ static bool __attribute__((unused)) configure_authorization(const char* root) {
     return false;
   }
   file_set_authorized_root(authorized_root_fd, authorized_root);
-  utils_set_authorized_root_fd(authorized_root_fd);
+  utils_set_authorized_root(authorized_root_fd, authorized_root);
   return true;
 }
 
@@ -363,10 +363,12 @@ int main(int argc, char* argv[]) {
     return 1;
   }
   if (stdio_mode) {
+    /* SSH authenticates the stdio transport outside of FastSync. */
+    allow_unauthenticated = true;
     io_set_fds(STDIN_FILENO, STDOUT_FILENO);
     handler(STDIN_FILENO);
     file_set_authorized_root(-1, NULL);
-    utils_set_authorized_root_fd(-1);
+    utils_set_authorized_root(-1, NULL);
     close(authorized_root_fd);
     free(authorized_root);
     return 0;

src/shared/chunk.c

@@ -1,4 +1,5 @@
 #include <stddef.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -17,17 +18,27 @@
 #define MAX_FILES_PER_CHUNK 65536U
 
 Chunk* chunk_create(File** items, int element_count) {
+  if (element_count < 0 || (element_count > 0 && items == NULL))
+    return NULL;
   Chunk* chunk = (Chunk*)malloc(sizeof(Chunk));
   if (chunk == NULL) {
     perror("ERROR: Could not allocate memory for chunk structure");
     return NULL;
   }
 
-  chunk->items = (File**)malloc(element_count * sizeof(File*));
+  if (element_count == 0) {
+    chunk->items = NULL;
+  } else {
+    if ((size_t)element_count > SIZE_MAX / sizeof(File*)) {
+      free(chunk);
+      return NULL;
+    }
+    chunk->items = (File**)malloc((size_t)element_count * sizeof(File*));
     if (chunk->items == NULL) {
       free(chunk);
       return NULL;
     }
+  }
 
   for (int i = 0; i < element_count; i++) {
     chunk->items[i] = items[i];
@@ -139,18 +150,25 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) {
 
     File* file = file_create(path);
     free(path);
+    if (file == NULL) {
+      array_list_delete(files);
+      return NULL;
+    }
 
     if (use_metadata) {
       if (remaining_size < sizeof(int)) {
         log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for metadata");
+        file_destroy(file);
         array_list_delete(files);
         return NULL;
       }
       // Peek at present flag to determine total size needed before reading
       int present_flag;
       memcpy(&present_flag, data_pointer, sizeof(int));
-      if (present_flag && remaining_size < sizeof(int) + FILE_METADATA_WIRE_SIZE) {
+      if ((present_flag != 0 && present_flag != 1) ||
+          (present_flag == 1 && remaining_size < sizeof(int) + FILE_METADATA_WIRE_SIZE)) {
         log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for metadata body");
+        file_destroy(file);
         array_list_delete(files);
         return NULL;
       }
@@ -162,6 +180,7 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) {
 
     if (remaining_size < sizeof(size_t)) {
       log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for data size");
+      file_destroy(file);
       array_list_delete(files);
       return NULL;
     }
@@ -173,6 +192,7 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) {
 
     if (remaining_size < file_data_size) {
       log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for file content");
+      file_destroy(file);
       array_list_delete(files);
       return NULL;
     }
@@ -181,19 +201,27 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) {
     if (file_data_size > MAX_FILE_DATA_SIZE) {
       log_message(LOG_LEVEL_ERROR, "File data size %zu exceeds maximum %llu", file_data_size,
                   (unsigned long long)MAX_FILE_DATA_SIZE);
+      file_destroy(file);
       array_list_delete(files);
       return NULL;
     }
 
-    void* file_data = malloc(file_data_size);
+    size_t allocation_size = file_data_size > 0 ? file_data_size : 1;
+    void* file_data = malloc(allocation_size);
     if (file_data == NULL) {
       perror("Could not allocate memory for file data");
+      file_destroy(file);
       array_list_delete(files);
       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) {
+      file_destroy(file);
+      array_list_delete(files);
+      return NULL;
+    }
     data_pointer += file_data_size;
     remaining_size -= file_data_size;
 
@@ -205,9 +233,14 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) {
   }
 
   File** file_array = (File**)array_list_to_array(files);
+  if (files->size > 0 && file_array == NULL) {
+    array_list_delete(files);
+    return NULL;
+  }
   Chunk* chunk = chunk_create(file_array, files->size);
 
   free(file_array);
+  if (chunk != NULL)
     files->item_destroyer = NULL;
   array_list_delete(files);
 

src/shared/metadata.c

@@ -175,7 +175,8 @@ FileMetadata* metadata_receive(int file_descriptor, int* ok) {
 void file_restore_metadata(const char* path, const FileMetadata* metadata) {
   if (metadata == NULL)
     return;
-  if (chmod(path, metadata->mode & 07777 & ~(S_ISUID | S_ISGID)) != 0)
+  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
      receiver write path; retain this legacy API only for compatibility. */
@@ -192,7 +193,8 @@ bool file_restore_metadata_fd(int fd, const FileMetadata* metadata) {
   if (fd < 0 || metadata == NULL)
     return metadata == NULL;
   bool ok = true;
-  if (fchmod(fd, metadata->mode & 07777 & ~(S_ISUID | S_ISGID)) != 0)
+  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. */
   struct timespec times[2] = {{.tv_sec = 0, .tv_nsec = UTIME_OMIT},

src/shared/transport_tls.c

@@ -35,6 +35,10 @@ static void log_ssl_errors(void) {
 
 static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key,
                                const char* ca_path) {
+  if (!is_server && !ca_path) {
+    log_message(LOG_LEVEL_ERROR, "TLS clients require a CA certificate path");
+    return NULL;
+  }
   const SSL_METHOD* method = is_server ? TLS_server_method() : TLS_client_method();
   SSL_CTX* ctx = SSL_CTX_new(method);
   if (!ctx) {
@@ -43,7 +47,10 @@ static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key
     return NULL;
   }
 
-  SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION);
+  if (SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION) != 1) {
+    SSL_CTX_free(ctx);
+    return NULL;
+  }
   if (SSL_CTX_set_cipher_list(ctx, "HIGH:!aNULL:!eNULL:!MD5:!RC4:!3DES") != 1) {
     SSL_CTX_free(ctx);
     return NULL;
@@ -103,7 +110,10 @@ static SSL* wrap_fd_with_ssl(int fd, SSL_CTX* ctx, bool is_server, const char* h
   // Enable hostname verification for client connections when a hostname is provided.
   // Must be done before SSL_connect to take effect during the handshake.
   if (!is_server && hostname) {
-    SSL_set1_host(ssl, hostname);
+    if (SSL_set1_host(ssl, hostname) != 1) {
+      SSL_free(ssl);
+      return NULL;
+    }
   }
 
   // Retry SSL_accept/SSL_connect on WANT_READ/WANT_WRITE (non-blocking handshake)

src/shared/utils.c

@@ -11,9 +11,58 @@
 #include <unistd.h>
 
 static int authorized_root_fd = -1;
+static char* authorized_root_path;
 
-void utils_set_authorized_root_fd(int fd) {
+void utils_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 root_len = strlen(root);
+  return strncmp(root, path, root_len) == 0 && (path[root_len] == '\0' || path[root_len] == '/');
+}
+
+static int open_authorized_destination(const char* dest_root) {
+  if (authorized_root_fd < 0 || !authorized_root_path || !dest_root ||
+      !path_is_within_root(authorized_root_path, dest_root))
+    return -1;
+
+  int dirfd = dup(authorized_root_fd);
+  if (dirfd < 0)
+    return -1;
+
+  const char* relative_path = dest_root + strlen(authorized_root_path);
+  while (*relative_path == '/')
+    relative_path++;
+  char* relative = str_dup(*relative_path ? relative_path : ".");
+  if (!relative) {
+    close(dirfd);
+    return -1;
+  }
+
+  char* saveptr = NULL;
+  char* component = strtok_r(relative, "/", &saveptr);
+  while (component) {
+    if (strcmp(component, ".") == 0 || strcmp(component, "..") == 0) {
+      free(relative);
+      close(dirfd);
+      return -1;
+    }
+    int next = openat(dirfd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
+    if (next < 0) {
+      free(relative);
+      close(dirfd);
+      return -1;
+    }
+    close(dirfd);
+    dirfd = next;
+    component = strtok_r(NULL, "/", &saveptr);
+  }
+
+  free(relative);
+  return dirfd;
 }
 
 bool mkdir_r(const char* path) {
@@ -208,7 +257,7 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes
 
 bool delete_extras(const char* dest_root, ArrayList* manifest) {
   int rootfd = authorized_root_fd >= 0
-                   ? dup(authorized_root_fd)
+                   ? open_authorized_destination(dest_root)
                    : open(dest_root, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
   if (rootfd < 0)
     return false;

src/shared/utils.h

@@ -9,7 +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);
+void utils_set_authorized_root(int fd, const char* canonical_path);
 bool has_path_traversal(const char* path);
 
 #endif