Backport of upstream commit 7712455d9aa33d0db0945effaa07e900b85987b1

Author: Miroslav Lichvar <mlichvar@redhat.com>
Date:   Wed Jul 31 15:01:15 2013 +0200

    Fix buffer overflow when processing crafted command packets
    
    When the length of the REQ_SUBNETS_ACCESSED, REQ_CLIENT_ACCESSES
    command requests and the RPY_SUBNETS_ACCESSED, RPY_CLIENT_ACCESSES,
    RPY_CLIENT_ACCESSES_BY_INDEX, RPY_MANUAL_LIST command replies is
    calculated, the number of items stored in the packet is not validated.
    
    A crafted command request/reply can be used to crash the server/client.
    Only clients allowed by cmdallow (by default only localhost) can crash
    the server.
    
    With chrony versions 1.25 and 1.26 this bug has a smaller security
    impact as the server requires the clients to be authenticated in order
    to process the subnet and client accesses commands. In 1.27 and 1.28,
    however, the invalid calculated length is included also in the
    authentication check which may cause another crash.

diff -up chrony-1.25/client.c.cve-2012-4502 chrony-1.25/client.c
--- chrony-1.25/client.c.cve-2012-4502	2013-08-09 12:50:40.858855754 +0200
+++ chrony-1.25/client.c	2013-08-09 13:02:52.306179674 +0200
@@ -1374,7 +1374,8 @@ submit_request(CMD_Request *request, CMD
         read_length = recvfrom_status;
         expected_length = PKL_ReplyLength(reply);
 
-        bad_length = (read_length != expected_length);
+        bad_length = (read_length != expected_length ||
+                      expected_length < offsetof(CMD_Reply, data));
         bad_sender = (where_from.u.sa_family != his_addr.u.sa_family ||
                       (where_from.u.sa_family == AF_INET &&
                        (where_from.in4.sin_addr.s_addr != his_addr.in4.sin_addr.s_addr ||
diff -up chrony-1.25/cmdmon.c.cve-2012-4502 chrony-1.25/cmdmon.c
--- chrony-1.25/cmdmon.c.cve-2012-4502	2011-05-04 12:29:40.000000000 +0200
+++ chrony-1.25/cmdmon.c	2013-08-09 12:50:40.861855759 +0200
@@ -1794,29 +1794,10 @@ read_from_cmd_socket(void *anything)
   }
 
   read_length = status;
-  expected_length = PKL_CommandLength(&rx_message);
-  rx_command = ntohs(rx_message.command);
 
   LCL_ReadRawTime(&now);
   LCL_CookTime(&now, &cooked_now, NULL);
 
-  tx_message.version = PROTO_VERSION_NUMBER;
-  tx_message.pkt_type = PKT_TYPE_CMD_REPLY;
-  tx_message.res1 = 0;
-  tx_message.res2 = 0;
-  tx_message.command = rx_message.command;
-  tx_message.sequence = rx_message.sequence;
-  tx_message.reply = htons(RPY_NULL);
-  tx_message.number = htons(1);
-  tx_message.total = htons(1);
-  tx_message.pad1 = 0;
-  tx_message.utoken = htonl(utoken);
-  /* Set this to a default (invalid) value.  This protects against the
-     token field being set to an arbitrary value if we reject the
-     message, e.g. due to the host failing the access check. */
-  tx_message.token = htonl(0xffffffffUL);
-  memset(&tx_message.auth, 0, sizeof(tx_message.auth));
-
   switch (where_from.u.sa_family) {
     case AF_INET:
       remote_ip.family = IPADDR_INET4;
@@ -1843,7 +1824,14 @@ read_from_cmd_socket(void *anything)
 
   allowed = ADF_IsAllowed(access_auth_table, &remote_ip) || localhost;
 
-  if (read_length < offsetof(CMD_Request, data) ||
+  /* Message size sanity check */
+  if (read_length >= offsetof(CMD_Request, data)) {
+    expected_length = PKL_CommandLength(&rx_message);
+  } else {
+    expected_length = 0;
+  }
+
+  if (expected_length < offsetof(CMD_Request, data) ||
       rx_message.pkt_type != PKT_TYPE_CMD_REQUEST ||
       rx_message.res1 != 0 ||
       rx_message.res2 != 0) {
@@ -1855,6 +1843,25 @@ read_from_cmd_socket(void *anything)
     return;
   }
 
+  rx_command = ntohs(rx_message.command);
+
+  tx_message.version = PROTO_VERSION_NUMBER;
+  tx_message.pkt_type = PKT_TYPE_CMD_REPLY;
+  tx_message.res1 = 0;
+  tx_message.res2 = 0;
+  tx_message.command = rx_message.command;
+  tx_message.sequence = rx_message.sequence;
+  tx_message.reply = htons(RPY_NULL);
+  tx_message.number = htons(1);
+  tx_message.total = htons(1);
+  tx_message.pad1 = 0;
+  tx_message.utoken = htonl(utoken);
+  /* Set this to a default (invalid) value.  This protects against the
+     token field being set to an arbitrary value if we reject the
+     message, e.g. due to the host failing the access check. */
+  tx_message.token = htonl(0xffffffffUL);
+  memset(&tx_message.auth, 0, sizeof(tx_message.auth));
+
   if (rx_message.version != PROTO_VERSION_NUMBER) {
     tx_message.status = htons(STT_NOHOSTACCESS);
     if (!LOG_RateLimited()) {
diff -up chrony-1.25/pktlength.c.cve-2012-4502 chrony-1.25/pktlength.c
--- chrony-1.25/pktlength.c.cve-2012-4502	2011-05-04 12:29:40.000000000 +0200
+++ chrony-1.25/pktlength.c	2013-08-09 12:50:40.863855762 +0200
@@ -125,6 +125,8 @@ PKL_CommandLength(CMD_Request *r)
         {
           unsigned long ns;
           ns = ntohl(r->data.subnets_accessed.n_subnets);
+          if (ns > MAX_SUBNETS_ACCESSED)
+            return 0;
           return (offsetof(CMD_Request, data.subnets_accessed.subnets) +
                   ns * sizeof(REQ_SubnetsAccessed_Subnet));
         }
@@ -132,6 +134,8 @@ PKL_CommandLength(CMD_Request *r)
         {
           unsigned long nc;
           nc = ntohl(r->data.client_accesses.n_clients);
+          if (nc > MAX_CLIENT_ACCESSES)
+            return 0;
           return (offsetof(CMD_Request, data.client_accesses.client_ips) +
                   nc * sizeof(unsigned long));
         }
@@ -195,6 +199,8 @@ PKL_ReplyLength(CMD_Reply *r)
         {
           unsigned long ns = ntohl(r->data.subnets_accessed.n_subnets);
           if (r->status == htons(STT_SUCCESS)) {
+            if (ns > MAX_SUBNETS_ACCESSED)
+              return 0;
             return (offsetof(CMD_Reply, data.subnets_accessed.subnets) +
                     ns * sizeof(RPY_SubnetsAccessed_Subnet));
           } else {
@@ -205,6 +211,8 @@ PKL_ReplyLength(CMD_Reply *r)
         {
           unsigned long nc = ntohl(r->data.client_accesses.n_clients);
           if (r->status == htons(STT_SUCCESS)) {
+            if (nc > MAX_CLIENT_ACCESSES)
+              return 0;
             return (offsetof(CMD_Reply, data.client_accesses.clients) +
                     nc * sizeof(RPY_ClientAccesses_Client));
           } else {
@@ -215,6 +223,8 @@ PKL_ReplyLength(CMD_Reply *r)
         {
           unsigned long nc = ntohl(r->data.client_accesses_by_index.n_clients);
           if (r->status == htons(STT_SUCCESS)) {
+            if (nc > MAX_CLIENT_ACCESSES)
+              return 0;
             return (offsetof(CMD_Reply, data.client_accesses_by_index.clients) +
                     nc * sizeof(RPY_ClientAccesses_Client));
           } else {
@@ -224,6 +234,8 @@ PKL_ReplyLength(CMD_Reply *r)
       case RPY_MANUAL_LIST:
         {
           unsigned long ns = ntohl(r->data.manual_list.n_samples);
+          if (ns > MAX_MANUAL_LIST_SAMPLES)
+            return 0;
           if (r->status == htons(STT_SUCCESS)) {
             return (offsetof(CMD_Reply, data.manual_list.samples) +
                     ns * sizeof(RPY_ManualListSample));