pktlength: handle truncated MANUAL_LIST reply

Before reading the n_samples field of the MANUAL_LIST reply, check if it
is actually contained in the received message. This does not change the
outcome of the client's length check as the returned length was always
larger than the length of the truncated reply and it was dropped anyway,
but it prevents the client from reading uninitialized memory.

diff --git a/client.c b/client.c
index 7d1e3465b3f8f47f46778fafe53ec79cd418706b..847afac86444a6dea8073c22fd19e6b6bccbfc10 100644 (file)
--- a/client.c
+++ b/client.c
@@ -1427,14 +1427,9 @@ submit_request(CMD_Request *request, CMD_Reply *reply)
         DEBUG_LOG("Received %d bytes", recv_status);
         
         read_length = recv_status;
-        if (read_length >= offsetof(CMD_Reply, data)) {
-          expected_length = PKL_ReplyLength(reply);
-        } else {
-          expected_length = 0;
-        }
+        expected_length = PKL_ReplyLength(reply, read_length);
 
-        bad_length = (read_length < expected_length ||
-                      expected_length < offsetof(CMD_Reply, data));
+        bad_length = !expected_length || read_length < expected_length;
         
         if (!bad_length) {
           bad_sequence = reply->sequence != request->sequence;

diff --git a/cmdmon.c b/cmdmon.c
index 4ed2189022c9ecda80e759104e9cb84cb7124279..283ba97181716d476a96aedd22190a6b0b4bf7f9 100644 (file)
--- a/cmdmon.c
+++ b/cmdmon.c
@@ -279,7 +279,7 @@ do_size_checks(void)
     reply.reply = htons(i);
     reply.status = STT_SUCCESS;
     reply.data.manual_list.n_samples = htonl(MAX_MANUAL_LIST_SAMPLES);
-    reply_length = PKL_ReplyLength(&reply);
+    reply_length = PKL_ReplyLength(&reply, sizeof (reply));
     if ((reply_length && reply_length < offsetof(CMD_Reply, data)) ||
         reply_length > sizeof (CMD_Reply))
       assert(0);
@@ -393,7 +393,7 @@ transmit_reply(CMD_Reply *msg, union sockaddr_all *where_to)
       assert(0);
   }
 
-  tx_message_length = PKL_ReplyLength(msg);
+  tx_message_length = PKL_ReplyLength(msg, sizeof (*msg));
   status = sendto(sock_fd, (void *) msg, tx_message_length, 0,
                   &where_to->sa, addrlen);
 

diff --git a/pktlength.c b/pktlength.c
index 23a1b474048eae89eb03d909679cde4ba0384dd7..d93d1390f1682953aff4de497563fecd514894b1 100644 (file)
--- a/pktlength.c
+++ b/pktlength.c
@@ -183,12 +183,15 @@ PKL_CommandPaddingLength(CMD_Request *r)
 /* ================================================== */
 
 int
-PKL_ReplyLength(CMD_Reply *r)
+PKL_ReplyLength(CMD_Reply *r, int read_length)
 {
   uint32_t type;
 
   assert(sizeof (reply_lengths) / sizeof (reply_lengths[0]) == N_REPLY_TYPES);
 
+  if (read_length < (int)offsetof(CMD_Reply, data))
+    return 0;
+
   type = ntohs(r->reply);
 
   /* Note that reply type codes start from 1, not 0 */
@@ -202,6 +205,9 @@ PKL_ReplyLength(CMD_Reply *r)
     if (r->status != htons(STT_SUCCESS))
       return offsetof(CMD_Reply, data);
 
+    if (read_length < (int)offsetof(CMD_Reply, data.manual_list.samples))
+      return 0;
+
     ns = ntohl(r->data.manual_list.n_samples);
     if (ns > MAX_MANUAL_LIST_SAMPLES)
       return 0;

diff --git a/pktlength.h b/pktlength.h
index fad4c30f730e4923d58e1a6860e1f33c58587bfb..40646fe1ed96d5774cc540ae13e8ef63ee82a7ec 100644 (file)
--- a/pktlength.h
+++ b/pktlength.h
@@ -35,6 +35,6 @@ extern int PKL_CommandLength(CMD_Request *r);
 
 extern int PKL_CommandPaddingLength(CMD_Request *r);
 
-extern int PKL_ReplyLength(CMD_Reply *r);
+extern int PKL_ReplyLength(CMD_Reply *r, int read_length);
 
 #endif /* GOT_PKTLENGTH_H */