From 2f4f096d80697d5a11097a822d24d21916e826d8 Mon Sep 17 00:00:00 2001
From: Levi Neely <lkn@darkstar.example.net>
Date: Tue, 3 Mar 2026 11:29:56 +0100
Subject: [PATCH] Fix input validation: stoull exception and buffer bounds

- Wrap std::stoull() in try/catch to prevent daemon crash on invalid folder ID
- Add proper read loop for RPC messages to handle partial reads
- Validate msg->length before memcpy to prevent heap over-read
- Handle daemon bugs gracefully (EOF before full message)
- Fix operator precedence in sync remove command

Fixes #205, #206
---
 control_tools.cpp |  3 ++-
 pclsync_lib.cpp   | 10 ++++++++-
 rpcclient.cpp     | 52 +++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 57 insertions(+), 8 deletions(-)

diff --git a/control_tools.cpp b/control_tools.cpp
index dc1b964..59d25f7 100644
--- a/control_tools.cpp
+++ b/control_tools.cpp
@@ -390,7 +390,8 @@ void setup_app(CLI::App *app) {
     const char *folderid = syncrm_fid.c_str();
 
     RpcClient *rpc = new RpcClient();
-    if(int result = rpc->Call(STOPSYNC, folderid, &errm, &errmsz) != 0) {
+    int result = rpc->Call(STOPSYNC, folderid, &errm, &errmsz);
+    if(result != 0) {
       std::cerr << "Remove Sync Folder failed: " << (errm ? errm : "no message") << std::endl;
       if (errm) { free(errm); }
       delete rpc;
diff --git a/pclsync_lib.cpp b/pclsync_lib.cpp
index 10daf32..736a7c6 100644
--- a/pclsync_lib.cpp
+++ b/pclsync_lib.cpp
@@ -613,7 +613,15 @@ int clib::pclsync_lib::add_sync_folder(const char *combined_path) {
 // path is the folderid to remove
 int clib::pclsync_lib::remove_sync_folder(const char *fid) {
   psync_folderid_t folderid;
-  folderid = static_cast<psync_folderid_t>(std::stoull(fid, nullptr, 10));
+  try {
+    folderid = static_cast<psync_folderid_t>(std::stoull(fid, nullptr, 10));
+  } catch (const std::invalid_argument &e) {
+    std::cerr << "Invalid folder ID: " << fid << std::endl;
+    return -1;
+  } catch (const std::out_of_range &e) {
+    std::cerr << "Folder ID out of range: " << fid << std::endl;
+    return -1;
+  }
   return psync_delete_sync_by_folderid(folderid);
 }
 
diff --git a/rpcclient.cpp b/rpcclient.cpp
index c9d89ac..b22e4ac 100644
--- a/rpcclient.cpp
+++ b/rpcclient.cpp
@@ -111,9 +111,28 @@ int RpcClient::readResponse(int fd, char **out, size_t *out_size) {
         return -1;
     }
 
-    ssize_t bytes_read = read(fd, msg, POVERLAY_BUFSIZE);
-    if (bytes_read <= 0) {
-        const char *error_msg = (bytes_read == 0) ? "Connection closed" : "Read error";
+    // Read header first to get message length
+    size_t header_size = offsetof(rpc_message_t, value);
+    ssize_t bytes_read = 0;
+    ssize_t total_read = 0;
+    
+    while (total_read < (ssize_t)header_size) {
+        bytes_read = read(fd, ((char*)msg) + total_read, header_size - total_read);
+        if (bytes_read <= 0) {
+            const char *error_msg = (bytes_read == 0) ? "Connection closed" : "Read error";
+            *out = strdup(error_msg);
+            *out_size = strlen(error_msg) + 1;
+            putil_wipe(msg, POVERLAY_BUFSIZE);
+            free(msg);
+            return -1;
+        }
+        total_read += bytes_read;
+    }
+
+    // Validate msg->length
+    size_t max_value_size = POVERLAY_BUFSIZE - header_size;
+    if (msg->length > max_value_size) {
+        const char *error_msg = "Message length exceeds buffer size";
         *out = strdup(error_msg);
         *out_size = strlen(error_msg) + 1;
         putil_wipe(msg, POVERLAY_BUFSIZE);
@@ -121,9 +140,30 @@ int RpcClient::readResponse(int fd, char **out, size_t *out_size) {
         return -1;
     }
 
-    *out = (char *)malloc(msg->length);
-    memcpy(*out, msg->value, msg->length);
-    *out_size = msg->length;
+    // Read the value payload
+    while (total_read < (ssize_t)(header_size + msg->length)) {
+        bytes_read = read(fd, ((char*)msg) + total_read, header_size + msg->length - total_read);
+        if (bytes_read < 0) {
+            const char *error_msg = "Read error";
+            *out = strdup(error_msg);
+            *out_size = strlen(error_msg) + 1;
+            putil_wipe(msg, POVERLAY_BUFSIZE);
+            free(msg);
+            return -1;
+        }
+        if (bytes_read == 0) {
+            // EOF before reading full message - daemon bug, but handle gracefully
+            break;
+        }
+        total_read += bytes_read;
+    }
+
+    // Use actual bytes read, not claimed length
+    size_t actual_length = total_read - header_size;
+
+    *out = (char *)malloc(actual_length);
+    memcpy(*out, msg->value, actual_length);
+    *out_size = actual_length;
 
     putil_wipe(msg, POVERLAY_BUFSIZE);
     free(msg);