ntpd plugin: Fix a possible buffer overflow.

[collectd.git] / src / ntpd.c

diff --git a/src/ntpd.c b/src/ntpd.c
index 40c2aa4..08c0c9f 100644 (file)
--- a/src/ntpd.c
+++ b/src/ntpd.c
@@ -69,12 +69,12 @@ static char *time_dispersion_file = "ntpd/time_dispersion-%s.rrd";
 static char *time_delay_file      = "ntpd/delay-%s.rrd";
 
 /* used for `time_offset', `time_dispersion', and `delay' */
-static char *ms_ds_def[] =
+static char *sec_ds_def[] =
 {
-       "DS:ms:GAUGE:"COLLECTD_HEARTBEAT":-1000000:1000000",
+       "DS:seconds:GAUGE:"COLLECTD_HEARTBEAT":-1000000:1000000",
        NULL
 };
-static int ms_ds_num = 1;
+static int sec_ds_num = 1;
 
 static char *frequency_offset_file = "ntpd/frequency_offset-%s.rrd";
 static char *frequency_offset_ds_def[] =
@@ -315,7 +315,7 @@ static void ntpd_init (void)
        return;
 }
 
-static void ntpd_write_ms (char *host, char *inst, char *val, char *file)
+static void ntpd_write_sec (char *host, char *inst, char *val, char *file)
 {
        char buf[256];
        int  status;
@@ -325,22 +325,22 @@ static void ntpd_write_ms (char *host, char *inst, char *val, char *file)
                return;
 
        rrd_update_file (host, buf, val,
-                       ms_ds_def, ms_ds_num);
+                       sec_ds_def, sec_ds_num);
 }
 
 static void ntpd_write_time_offset (char *host, char *inst, char *val)
 {
-       ntpd_write_ms (host, inst, val, time_offset_file);
+       ntpd_write_sec (host, inst, val, time_offset_file);
 }
 
 static void ntpd_write_time_dispersion (char *host, char *inst, char *val)
 {
-       ntpd_write_ms (host, inst, val, time_dispersion_file);
+       ntpd_write_sec (host, inst, val, time_dispersion_file);
 }
 
 static void ntpd_write_delay (char *host, char *inst, char *val)
 {
-       ntpd_write_ms (host, inst, val, time_delay_file);
+       ntpd_write_sec (host, inst, val, time_delay_file);
 }
 
 static void ntpd_write_frequency_offset (char *host, char *inst, char *val)
@@ -560,6 +560,9 @@ static int ntpd_receive_response (int req_code, int *res_items, int *res_size,
                if (status < 0)
                {
                        DBG ("recv(2) failed: %s", strerror (errno));
+                       DBG ("Closing socket #%i", sd);
+                       close (sd);
+                       sock_descr = sd = -1;
                        return (-1);
                }
 
@@ -628,6 +631,14 @@ static int ntpd_receive_response (int req_code, int *res_items, int *res_size,
                        continue;
                }
 
+               if (pkt_item_len > res_item_size)
+               {
+                       syslog (LOG_ERR, "ntpd plugin: (pkt_item_len = %i) "
+                                       ">= (res_item_size = %i)",
+                                       pkt_item_len, res_item_size);
+                       continue;
+               }
+
                /* If this is the first packet (time wise, not sequence wise),
                 * set `res_size'. If it's not the first packet check if the
                 * items have the same size. Discard invalid packets. */
@@ -644,9 +655,16 @@ static int ntpd_receive_response (int req_code, int *res_items, int *res_size,
                        continue;
                }
 
+               /*
+                * Because the items in the packet may be smaller than the
+                * items requested, the following holds true:
+                */
+               assert ((*res_size == pkt_item_len)
+                               && (pkt_item_len <= res_item_size));
+
                /* Calculate the padding. No idea why there might be any padding.. */
                pkt_padding = 0;
-               if (res_item_size > pkt_item_len)
+               if (pkt_item_len < res_item_size)
                        pkt_padding = res_item_size - pkt_item_len;
                DBG ("res_item_size = %i; pkt_padding = %i;",
                                res_item_size, pkt_padding);
@@ -697,11 +715,20 @@ static int ntpd_receive_response (int req_code, int *res_items, int *res_size,
                        syslog (LOG_ERR, "ntpd plugin: realloc failed.");
                        continue;
                }
+               items_num += pkt_item_num;
                *res_data = items;
 
                for (i = 0; i < pkt_item_num; i++)
                {
+                       /* dst: There are already `*res_items' items with
+                        *      res_item_size bytes each in in `*res_data'. Set
+                        *      dst to the first byte after that. */
                        void *dst = (void *) (*res_data + ((*res_items) * res_item_size));
+                       /* src: We use `pkt_item_len' to calculate the offset
+                        *      from the beginning of the packet, because the
+                        *      items in the packet may be smaller than the
+                        *      items that were requested. We skip `i' such
+                        *      items. */
                        void *src = (void *) (((char *) res.data) + (i * pkt_item_len));
 
                        /* Set the padding to zeros */
@@ -709,8 +736,10 @@ static int ntpd_receive_response (int req_code, int *res_items, int *res_size,
                                memset (dst, '\0', res_item_size);
                        memcpy (dst, src, (size_t) pkt_item_len);
 
+                       /* Increment `*res_items' by one, so `dst' will end up
+                        * one further in the next round. */
                        (*res_items)++;
-               }
+               } /* for (pkt_item_num) */
 
                pkt_recvd[pkt_sequence] = (char) 1;
                pkt_recvd_num++;
@@ -720,7 +749,7 @@ static int ntpd_receive_response (int req_code, int *res_items, int *res_size,
        } /* while (done == 0) */
 
        return (0);
-}
+} /* int ntpd_receive_response */
 
 /* For a description of the arguments see `ntpd_do_query' below. */
 static int ntpd_send_request (int req_code, int req_items, int req_size, char *req_data)
@@ -758,7 +787,12 @@ static int ntpd_send_request (int req_code, int req_items, int req_size, char *r
 
        status = swrite (sd, (const char *) &req, REQ_LEN_NOMAC);
        if (status < 0)
+       {
+               DBG ("`swrite' failed. Closing socket #%i", sd);
+               close (sd);
+               sock_descr = sd = -1;
                return (status);
+       }
 
        return (0);
 }
@@ -873,7 +907,7 @@ static void ntpd_read (void)
                struct info_peer_summary *ptr;
                double offset;
 
-               char peername[512];
+               char peername[NI_MAXHOST];
                int refclock_id;
                
                ptr = ps + i;
@@ -889,8 +923,15 @@ static void ntpd_read (void)
 
                if (ptr->v6_flag)
                {
-                       status = getnameinfo ((const struct sockaddr *) &ptr->srcadr6,
-                                       sizeof (ptr->srcadr6),
+                       struct sockaddr_in6 sa;
+
+                       memset (&sa, 0, sizeof (sa));
+                       sa.sin6_family = AF_INET6;
+                       sa.sin6_port = htons (123);
+                       memcpy (&sa.sin6_addr, &ptr->srcadr6, sizeof (struct in6_addr));
+
+                       status = getnameinfo ((const struct sockaddr *) &sa,
+                                       sizeof (sa),
                                        peername, sizeof (peername),
                                        NULL, 0, 0 /* no flags */);
                        if (status != 0)