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
This commit is contained in:
Levi Neely 2026-03-03 11:29:56 +01:00
parent 175a46a109
commit 2f4f096d80
3 changed files with 57 additions and 8 deletions

View File

@ -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;

View File

@ -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);
}

View File

@ -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);