ply-boot-client: validate reply sizes for both answer types

The client trusted the uint32 size field from daemon replies when reading
both ANSWER and MULTIPLE_ANSWERS responses:

  - For ANSWER: computing (size + 1) in uint32_t arithmetic wraps when
    size == UINT32_MAX, producing malloc(0) followed by a ~4 GiB read
    attempt into that tiny buffer.

  - For MULTIPLE_ANSWERS: the code asserted size > 0 rather than
    returning an error, did not cap the value before malloc(size), and
    walked the payload with a uint8_t index that wraps forever once the
    payload exceeds 255 bytes.

Reject size > 1 MiB for both reply types before allocating, so the
(size + 1) add can no longer overflow and oversized mallocs are avoided.
A zero-length ANSWER remains valid (it yields an empty string), while
MULTIPLE_ANSWERS additionally requires a non-zero payload that ends in a
NUL terminator and is walked with a uint32_t index.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

diff --git a/src/client/ply-boot-client.c b/src/client/ply-boot-client.c
index 2834940e9cec4a9547013b5cf5c46aa357c062fb..d7495cdb64a19963902b544855c4627bc27a4dd7 100644 (file)
--- a/src/client/ply-boot-client.c
+++ b/src/client/ply-boot-client.c
@@ -35,6 +35,12 @@
 #include "ply-logger.h"
 #include "ply-utils.h"
 
+/* Upper bound on the length of a reply payload accepted from the daemon.
+ * The daemon is trusted, but the length prefix is validated to avoid an
+ * integer overflow in the (size + 1) allocation and to reject absurdly
+ * large reads. 1 MiB is far larger than any legitimate reply. */
+#define PLY_BOOT_CLIENT_MAX_ANSWER_SIZE (1024 * 1024)
+
 struct _ply_boot_client
 {
         ply_event_loop_t                    *loop;
@@ -279,6 +285,9 @@ ply_boot_client_process_incoming_replies (ply_boot_client_t *client)
                 if (!ply_read_uint32 (client->socket_fd, &size))
                         goto out;
 
+                if (size > PLY_BOOT_CLIENT_MAX_ANSWER_SIZE)
+                        goto out;
+
                 answer = malloc ((size + 1) * sizeof(char));
                 if (size > 0) {
                         if (!ply_read (client->socket_fd, answer, size)) {
@@ -297,7 +306,7 @@ ply_boot_client_process_incoming_replies (ply_boot_client_t *client)
                 char *answer;
                 char *p;
                 char *q;
-                uint8_t i;
+                uint32_t i;
 
                 array = NULL;
                 answers = NULL;
@@ -305,7 +314,8 @@ ply_boot_client_process_incoming_replies (ply_boot_client_t *client)
                 if (!ply_read_uint32 (client->socket_fd, &size))
                         goto out;
 
-                assert (size > 0);
+                if (size == 0 || size > PLY_BOOT_CLIENT_MAX_ANSWER_SIZE)
+                        goto out;
 
                 answer = malloc (size);
 
@@ -314,6 +324,14 @@ ply_boot_client_process_incoming_replies (ply_boot_client_t *client)
                         goto out;
                 }
 
+                /* The payload must be a sequence of NUL-terminated strings, so
+                 * require a trailing NUL; this keeps every strdup below in
+                 * bounds and rejects a malformed final segment. */
+                if (answer[size - 1] != '\0') {
+                        free (answer);
+                        goto out;
+                }
+
                 array = ply_array_new (PLY_ARRAY_ELEMENT_TYPE_POINTER);
 
                 p = answer;