Fix pcl-zqv.4.2: protocol parsing and buffer safety in prpc/papi (#348)

prpc.c:
- on_request: fix total_size computation. The original formula
  sizeof(uint32_t)+sizeof(uint64_t)+response->length had two bugs:
  (1) sizeof(uint32_t)+sizeof(uint64_t)=12 but offsetof(rpc_message_t,value)=16
  due to struct alignment padding between the uint32_t type and uint64_t length
  fields; and (2) respond() was storing full message size in response->length,
  double-counting the header. Fix: use offsetof(rpc_message_t,value)+response->length,
  consistent with readResponse() in rpcclient.cpp which reads a fixed header_size
  of offsetof(rpc_message_t,value) bytes then reads msg->length payload bytes.
- respond: store payload length only in response->length (value_length+1),
  not full message size, to match the client protocol expectation.
- prpc_init: add null check on malloc return value.
- prpc_register: restore old handler table and return -1 if prpc_init fails.

papi.c:
- papi_result_thread: add missing MAX_API_RESPONSE_SIZE guard (present in
  papi_result but absent here), preventing server-controlled unbounded malloc.
- papi_result, papi_result_thread: add null checks on malloc before passing
  pointer to psock_readall.
- papi_result_async: add MAX_API_RESPONSE_SIZE check and malloc null check
  on reader->respsize path.
- calc_ret_len: add _NEED_DATA(1) guard before ARRAY and HASH while-loop
  conditions; empty containers with datalen=0 caused out-of-bounds read.

Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

pclsync/papi.c

@@ -201,6 +201,7 @@ static ssize_t calc_ret_len(unsigned char **restrict data,
     int unsigned cnt;
     cnt = 0;
     ret = sizeof(binresult);
+    _NEED_DATA(1);
     while (**data != RPARAM_END) {
       r = calc_ret_len(data, datalen, strcnt);
       if (r == -1)
@@ -218,6 +219,7 @@ static ssize_t calc_ret_len(unsigned char **restrict data,
     int unsigned cnt;
     cnt = 0;
     ret = sizeof(binresult);
+    _NEED_DATA(1);
     while (**data != RPARAM_END) {
       r = calc_ret_len(data, datalen, strcnt);
       if (r == -1)
@@ -405,6 +407,10 @@ binresult *papi_result(psock_t *sock) {
   }
 
   data = (unsigned char *)malloc(ressize);
+  if (!data) {
+    pdbg_logf(D_ERROR, "Failed to allocate %u bytes for API response", ressize);
+    return NULL;
+  }
 
   if (pdbg_unlikely(psock_readall(sock, data, ressize) != ressize)) {
     free(data);
@@ -424,7 +430,16 @@ binresult *papi_result_thread(psock_t *sock) {
   if (pdbg_unlikely(psock_readall_thread(
                        sock, &ressize, sizeof(uint32_t)) != sizeof(uint32_t)))
     return NULL;
+  if (ressize > MAX_API_RESPONSE_SIZE) {
+    pdbg_logf(D_WARNING, "API response size %u exceeds limit %u, rejecting",
+              ressize, MAX_API_RESPONSE_SIZE);
+    return NULL;
+  }
   data = (unsigned char *)malloc(ressize);
+  if (!data) {
+    pdbg_logf(D_ERROR, "Failed to allocate %u bytes for API response", ressize);
+    return NULL;
+  }
   if (pdbg_unlikely(psock_readall_thread(sock, data, ressize) !=
                    ressize)) {
     free(data);
@@ -467,8 +482,17 @@ again:
       reader->state = 1;
       reader->bytesread = 0;
       reader->bytestoread = reader->respsize;
+      if (reader->respsize > MAX_API_RESPONSE_SIZE) {
+        pdbg_logf(D_WARNING, "API response size %u exceeds limit %u, rejecting",
+                  reader->respsize, MAX_API_RESPONSE_SIZE);
+        reader->result = NULL;
+        papi_rdr_alloc(reader);
+        return ASYNC_RES_READY;
+      }
       reader->data = (unsigned char *)malloc(reader->respsize);
       if (!reader->data) {
+        pdbg_logf(D_ERROR, "Failed to allocate %u bytes for API response",
+                  reader->respsize);
         reader->result = NULL;
         papi_rdr_alloc(reader);
         return ASYNC_RES_READY;

pclsync/prpc.c

@@ -36,6 +36,7 @@
 */
 
 #include <errno.h>
+#include <stddef.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -108,7 +109,7 @@ static void on_request(void *lpvParam) {
   if (request) {
     respond(request, response);
 
-    ssize_t total_size = sizeof(uint32_t) + sizeof(uint64_t) + response->length;
+    ssize_t total_size = offsetof(rpc_message_t, value) + response->length;
     ssize_t bytes_written = write(*sockfd, response, total_size);
 
     if (bytes_written == -1) {
@@ -229,7 +230,7 @@ static void respond(rpc_message_t *request, rpc_message_t *response) {
     response->value[value_length] = '\0';
     pdbg_logf(D_WARNING, "Response message truncated to fit buffer");
   }
-  response->length = sizeof(rpc_message_t) + value_length + 1;
+  response->length = value_length + 1;
 }
 
 void prpc_main_loop() {
@@ -297,6 +298,11 @@ int prpc_register(int cmdid, prpc_handler h) {
   if (cmdid > (calbacks_lower_band + handlers_size)) {
     handlers_size = cmdid - calbacks_lower_band + 1;
     prpc_init();
+    if (!handlers) {
+      handlers = handlers_old;
+      handlers_size = handlers_size_old;
+      return -1;
+    }
     memcpy(handlers, handlers_old,
            handlers_size_old * sizeof(prpc_handler));
     free(handlers_old);
@@ -307,6 +313,10 @@ int prpc_register(int cmdid, prpc_handler h) {
 
 void prpc_init() {
   handlers = (prpc_handler *)malloc(sizeof(prpc_handler) * handlers_size);
+  if (!handlers) {
+    pdbg_logf(D_ERROR, "Failed to allocate handler table");
+    return;
+  }
   memset(handlers, 0, sizeof(prpc_handler) * handlers_size);
 }