1 files changed, 19 insertions, 20 deletions

diff --git a/plugins/check_ntp.c b/plugins/check_ntp.c
index 09a923eb..914b40ce 100644
--- a/plugins/check_ntp.c
+++ b/plugins/check_ntp.c
@@ -297,7 +297,7 @@ void setup_request(ntp_message *p){
  * this is done by filtering servers based on stratum, dispersion, and
  * finally round-trip delay. */
 int best_offset_server(const ntp_server_results *slist, int nservers){
-        int i=0, cserver=0, best_server=-1;
+        int cserver=0, best_server=-1;
 
         /* for each server */
         for(cserver=0; cserver<nservers; cserver++){
@@ -356,7 +356,7 @@ int best_offset_server(const ntp_server_results *slist, int nservers){
  *   we have to do it in a way that our lazy macros don't handle currently :( */
 double offset_request(const char *host, int *status){
         int i=0, j=0, ga_result=0, num_hosts=0, *socklist=NULL, respnum=0;
-        int servers_completed=0, one_written=0, one_read=0, servers_readable=0, best_index=-1;
+        int servers_completed=0, one_read=0, servers_readable=0, best_index=-1;
         time_t now_time=0, start_ts=0;
         ntp_message *req=NULL;
         double avg_offset=0.;
@@ -421,7 +421,6 @@ double offset_request(const char *host, int *status){
                  * been touched in the past second or so and is still lacking
                  * some responses.  for each of these servers, send a new request,
                  * and update the "waiting" timestamp with the current time. */
-                one_written=0;
                 now_time=time(NULL);
 
                 for(i=0; i<num_hosts; i++){
@@ -431,7 +430,6 @@ double offset_request(const char *host, int *status){
                                 setup_request(&req[i]);
                                 write(socklist[i], &req[i], sizeof(ntp_message));
                                 servers[i].waiting=now_time;
-                                one_written=1;
                                 break;
                         }
                 }
@@ -517,14 +515,13 @@ setup_control_request(ntp_control_message *p, uint8_t opcode, uint16_t seq){
 double jitter_request(const char *host, int *status){
         int conn=-1, i, npeers=0, num_candidates=0, syncsource_found=0;
         int run=0, min_peer_sel=PEER_INCLUDED, num_selected=0, num_valid=0;
-        int peers_size=0, peer_offset=0, bytes_read=0;
+        int peers_size=0, peer_offset=0;
         ntp_assoc_status_pair *peers=NULL;
         ntp_control_message req;
         const char *getvar = "jitter";
         double rval = 0.0, jitter = -1.0;
         char *startofvalue=NULL, *nptr=NULL;
         void *tmp;
-        int ntp_cm_ints = sizeof(uint16_t) * 5 + sizeof(uint8_t) * 2;
 
         /* Long-winded explanation:
          * Getting the jitter requires a number of steps:
@@ -551,7 +548,7 @@ double jitter_request(const char *host, int *status){
                 DBG(print_ntp_control_message(&req));
                 /* Attempt to read the largest size packet possible */
                 req.count=htons(MAX_CM_SIZE);
-                DBG(printf("recieving READSTAT response"))
+                DBG(printf("receiving READSTAT response"))
                 read(conn, &req, SIZEOF_NTPCM(req));
                 DBG(print_ntp_control_message(&req));
                 /* Each peer identifier is 4 bytes in the data section, which
@@ -591,6 +588,9 @@ double jitter_request(const char *host, int *status){
                 for (i = 0; i < npeers; i++){
                         /* Only query this server if it is the current sync source */
                         if (PEER_SEL(peers[i].status) >= min_peer_sel){
+                                char jitter_data[MAX_CM_SIZE+1];
+                                size_t jitter_data_count;
+
                                 num_selected++;
                                 setup_control_request(&req, OP_READVAR, 2);
                                 req.assoc = peers[i].assoc;
@@ -608,16 +608,8 @@ double jitter_request(const char *host, int *status){
                                 DBG(print_ntp_control_message(&req));
 
                                 req.count = htons(MAX_CM_SIZE);
-                                DBG(printf("recieving READVAR response...\n"));
-
-                                /* cov-66524 - req.data not null terminated before usage. Also covers verifying struct was returned correctly*/
-                                if ((bytes_read = read(conn, &req, SIZEOF_NTPCM(req))) == -1)
-                                        die(STATE_UNKNOWN, _("Cannot read from socket: %s"), strerror(errno));
-                                if (bytes_read != ntp_cm_ints + req.count)
-                                        die(STATE_UNKNOWN, _("Invalid NTP response: %d bytes read does not equal %d plus %d data segment"), bytes_read, ntp_cm_ints, req.count); 
-                                /* else null terminate */
-                                strncpy(req.data[req.count], "\0", 1);
-
+                                DBG(printf("receiving READVAR response...\n"));
+                                read(conn, &req, SIZEOF_NTPCM(req));
                                 DBG(print_ntp_control_message(&req));
 
                                 if(req.op&REM_ERROR && strstr(getvar, "jitter")) {
@@ -632,7 +624,14 @@ double jitter_request(const char *host, int *status){
                                 if(verbose) {
                                         printf("parsing jitter from peer %.2x: ", ntohs(peers[i].assoc));
                                 }
-                                startofvalue = strchr(req.data, '=');
+                                if((jitter_data_count = ntohs(req.count)) >= sizeof(jitter_data)){
+                                        die(STATE_UNKNOWN,
+                                            _("jitter response too large (%lu bytes)\n"),
+                                            (unsigned long)jitter_data_count);
+                                }
+                                memcpy(jitter_data, req.data, jitter_data_count);
+                                jitter_data[jitter_data_count] = '\0';
+                                startofvalue = strchr(jitter_data, '=');
                                 if(startofvalue != NULL) {
                                         startofvalue++;
                                         jitter = strtod(startofvalue, &nptr);
@@ -690,11 +689,11 @@ int process_arguments(int argc, char **argv){
                 switch (c) {
                 case 'h':
                         print_help();
-                        exit(STATE_OK);
+                        exit(STATE_UNKNOWN);
                         break;
                 case 'V':
                         print_revision(progname, NP_VERSION);
-                        exit(STATE_OK);
+                        exit(STATE_UNKNOWN);
                         break;
                 case 'v':
                         verbose++;