From 9ce73696b0407b43bcd96269fb1fd6c343834475 Mon Sep 17 00:00:00 2001 From: Spenser Reinhardt Date: Thu, 5 Jun 2014 22:43:07 -0500 Subject: plugins/check_apt.c - Print uninitialized ereg Coverity 66531 - ereg.buffer can be printed without being initialized if do_include and do_exclude are null and critical is an invalid regex. While minor this may leak memory and cause undefined behavior. diff --git a/plugins/check_apt.c b/plugins/check_apt.c index 4c76a51..07622c2 100644 --- a/plugins/check_apt.c +++ b/plugins/check_apt.c @@ -223,6 +223,9 @@ int run_upgrade(int *pkgcount, int *secpkgcount){ regex_t ireg, ereg, sreg; char *cmdline=NULL, rerrbuf[64]; + /* initialize ereg as it is possible it is printed while uninitialized */ + memset(&ereg, "\0", sizeof(ereg.buffer)); + if(upgrade==NO_UPGRADE) return STATE_OK; /* compile the regexps */ -- cgit v0.10-9-g596f From b61f51ad0291cf7051b6ea15ec8f8486f02443f9 Mon Sep 17 00:00:00 2001 From: Spenser Reinhardt Date: Thu, 5 Jun 2014 23:01:35 -0500 Subject: plugins/check_real.c - recv string null terminate Recv into buffer is not properly null terminated prior to strstr and possible other string functions expecting a null termination. Simply take bytes received and use as an index to append \0 after. We are creating buffer[] with size of MAX_INPUT_BUFFER and recv with MAX_INPUT_BUFFER-1 so this should never overflow. diff --git a/plugins/check_real.c b/plugins/check_real.c index 47776c5..36f6413 100644 --- a/plugins/check_real.c +++ b/plugins/check_real.c @@ -178,6 +178,7 @@ main (int argc, char **argv) /* watch for the REAL connection string */ result = recv (sd, buffer, MAX_INPUT_BUFFER - 1, 0); + buffer[result] = "\0"; /* null terminate recieved buffer */ /* return a CRITICAL status if we couldn't read any data */ if (result == -1) { -- cgit v0.10-9-g596f From a04df3e1b67dc5eab3adc202cc89901f801cdeaa Mon Sep 17 00:00:00 2001 From: Spenser Reinhardt Date: Sun, 22 Jun 2014 14:49:25 -0500 Subject: plugins/check_ntp.c - Verify struct from response Coverity 66524 - req.data is not neccessarily null terminated but still feed to printf statements. This both does that, and verifies the struct more so than before. - SR diff --git a/plugins/check_ntp.c b/plugins/check_ntp.c index 0a7640a..09a923e 100644 --- a/plugins/check_ntp.c +++ b/plugins/check_ntp.c @@ -517,13 +517,14 @@ 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; + int peers_size=0, peer_offset=0, bytes_read=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: @@ -608,7 +609,15 @@ double jitter_request(const char *host, int *status){ req.count = htons(MAX_CM_SIZE); DBG(printf("recieving READVAR response...\n")); - read(conn, &req, SIZEOF_NTPCM(req)); + + /* 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(print_ntp_control_message(&req)); if(req.op&REM_ERROR && strstr(getvar, "jitter")) { -- cgit v0.10-9-g596f From 5866cb0a09876d6b2a84006bda8aa9de7ea467fd Mon Sep 17 00:00:00 2001 From: Spenser Reinhardt Date: Sun, 22 Jun 2014 15:34:25 -0500 Subject: plugins/check_http.c - leakage fix Coverity 66514 - Possible leakage and overflow with addr in redirect functionality. Not confirmed as null terminated, and externally gathered. Restrict string comparisons and duplications by size. - SR diff --git a/plugins/check_http.c b/plugins/check_http.c index 92861d9..5167997 100644 --- a/plugins/check_http.c +++ b/plugins/check_http.c @@ -1243,6 +1243,7 @@ redir (char *pos, char *status_line) if (addr == NULL) die (STATE_UNKNOWN, _("HTTP UNKNOWN - Could not allocate addr\n")); + memset(addr, 0, MAX_IPV4_HOSTLENGTH); url = malloc (strcspn (pos, "\r\n")); if (url == NULL) die (STATE_UNKNOWN, _("HTTP UNKNOWN - Could not allocate URL\n")); @@ -1333,8 +1334,8 @@ redir (char *pos, char *status_line) max_depth, type, addr, i, url, (display_html ? "" : "")); if (server_port==i && - !strcmp(server_address, addr) && - (host_name && !strcmp(host_name, addr)) && + !strncmp(server_address, addr, MAX_IPV4_HOSTLENGTH) && + (host_name && !strncmp(host_name, addr, MAX_IPV4_HOSTLENGTH)) && !strcmp(server_url, url)) die (STATE_WARNING, _("HTTP WARNING - redirection creates an infinite loop - %s://%s:%d%s%s\n"), @@ -1343,11 +1344,11 @@ redir (char *pos, char *status_line) strcpy (server_type, type); free (host_name); - host_name = strdup (addr); + host_name = strndup (addr, MAX_IPV4_HOSTLENGTH); if (!(followsticky & STICKY_HOST)) { free (server_address); - server_address = strdup (addr); + server_address = strndup (addr, MAX_IPV4_HOSTLENGTH); } if (!(followsticky & STICKY_PORT)) { server_port = i; @@ -1366,6 +1367,7 @@ redir (char *pos, char *status_line) printf (_("Redirection to %s://%s:%d%s\n"), server_type, host_name ? host_name : server_address, server_port, server_url); + free(addr); check_http (); } -- cgit v0.10-9-g596f From e7e6edb2f8e43085d02cdda93fe16256ab3a35fe Mon Sep 17 00:00:00 2001 From: Spenser Reinhardt Date: Sun, 22 Jun 2014 16:02:19 -0500 Subject: plugins-root/check_dhcp.c - array out of bounds Coverity 66488 - offer_packet->options has a max size of 312. It was being used in a loop verifying less than 311, but increasing by 2 per loop, causing a possible array index out of bounds. Changed to checking less than max length - 1. - SR diff --git a/plugins-root/check_dhcp.c b/plugins-root/check_dhcp.c index 1ec5c39..b69a10d 100644 --- a/plugins-root/check_dhcp.c +++ b/plugins-root/check_dhcp.c @@ -837,7 +837,7 @@ int add_dhcp_offer(struct in_addr source,dhcp_packet *offer_packet){ return ERROR; /* process all DHCP options present in the packet */ - for(x=4;xoptions[x]==-1) break; -- cgit v0.10-9-g596f From 88472d1804d3cd42e0ea8717d75191dfb3e3bbeb Mon Sep 17 00:00:00 2001 From: Spenser Reinhardt Date: Sun, 22 Jun 2014 22:59:03 -0500 Subject: plugins/negate.c - Reorder if statement, aiob Coverity 66480 - Potential array index out of bounds, since result was not verified to be positive prior to using as an index for state[]. Simply reording the if statement should resolve the issue. - SR diff --git a/plugins/negate.c b/plugins/negate.c index 4bd09de..7787d01 100644 --- a/plugins/negate.c +++ b/plugins/negate.c @@ -98,8 +98,7 @@ main (int argc, char **argv) die (max_state_alt (result, STATE_UNKNOWN), _("No data returned from command\n")); for (i = 0; i < chld_out.lines; i++) { - if (subst_text && result != state[result] && - result >= 0 && result <= 4) { + if (subst_text && result >= 0 && result <= 4 && result != state[result]) { /* Loop over each match found */ while ((sub = strstr (chld_out.line[i], state_text (result)))) { /* Terminate the first part and skip over the string we'll substitute */ -- cgit v0.10-9-g596f From aa16beb9711c1a235259401e8883f5d807a0a11d Mon Sep 17 00:00:00 2001 From: Spenser Reinhardt Date: Sun, 22 Jun 2014 23:10:50 -0500 Subject: plugins/negate.c - Function should not return. Coverity 66479 - validate_arguments has no need to return anything, as it dies on error, yet was set to return an int. Set to void to resolve warning. diff --git a/plugins/negate.c b/plugins/negate.c index 7787d01..d512e34 100644 --- a/plugins/negate.c +++ b/plugins/negate.c @@ -44,7 +44,7 @@ const char *email = "devel@monitoring-plugins.org"; /* char *command_line; */ static const char **process_arguments (int, char **); -int validate_arguments (char **); +void validate_arguments (char **); void print_help (void); void print_usage (void); int subst_text = FALSE; @@ -205,7 +205,7 @@ process_arguments (int argc, char **argv) } -int +void validate_arguments (char **command_line) { if (command_line[0] == NULL) -- cgit v0.10-9-g596f From 9123f6146c5dd3285d8fb78cf3a8cd52bad17ec1 Mon Sep 17 00:00:00 2001 From: Spenser Reinhardt Date: Mon, 23 Jun 2014 13:54:39 -0500 Subject: lib/utils_cmd.c - Free file descriptor Coverity 66502 - File descriptor fd in cmd_file_read is never closed, and thus file is left open after usage throughout runtime. - SR diff --git a/lib/utils_cmd.c b/lib/utils_cmd.c index 4c6d0be..9e214bd 100644 --- a/lib/utils_cmd.c +++ b/lib/utils_cmd.c @@ -390,6 +390,9 @@ cmd_file_read ( char *filename, output *out, int flags) if(out) out->lines = _cmd_fetch_output (fd, out, flags); + + if (close(fd) == -1) + die( STATE_UNKNOWN, _("Error closing %s: %s"), filename, strerror(errno) ); return 0; } -- cgit v0.10-9-g596f