From cdd0a5f3f379627cd91ddf2cd597b30d11c5795b Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Mon, 13 May 2013 20:38:39 -0400 Subject: [PATCH 1/7] [server] minor memory leak bug fix during access.conf parsing found by Coverity --- server/access.c | 1 + 1 file changed, 1 insertion(+) diff --git a/server/access.c b/server/access.c index d690e2fd..88ca7fd8 100644 --- a/server/access.c +++ b/server/access.c @@ -1392,6 +1392,7 @@ acc_check_port_access(acc_stanza_t *acc, char *port_str) if(add_port_list_ent(&in_pl, buf) == 0) { log_msg(LOG_ERR, "Invalid proto/port string"); + free_acc_port_list(in_pl); return 0; } From 0c3da4bee4126ab96cabf35f45d2d02751d9e543 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Mon, 13 May 2013 20:40:29 -0400 Subject: [PATCH 2/7] [server] minor cosmetic (unnecessary NULL checks and one un-triggerable memory leak) found by Coverity --- server/fw_util_iptables.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/server/fw_util_iptables.c b/server/fw_util_iptables.c index 29a7cb7b..60f6deaa 100644 --- a/server/fw_util_iptables.c +++ b/server/fw_util_iptables.c @@ -707,7 +707,14 @@ process_spa_request(const fko_srv_options_t * const opts, /* Parse and expand our access message. */ if(expand_acc_port_list(&port_list, spadat->spa_message_remain) != 1) + { + /* technically we would already have exited with an error if there were + * any memory allocation errors (see the add_port_list() function), but + * for completeness... + */ + free_acc_port_list(port_list); return res; + } /* Start at the top of the proto-port list... */ @@ -740,7 +747,7 @@ process_spa_request(const fko_srv_options_t * const opts, if(jump_rule_exists(IPT_INPUT_ACCESS) == 0) add_jump_rule(opts, IPT_INPUT_ACCESS); - if(out_chain->to_chain != NULL && strlen(out_chain->to_chain)) + if(strlen(out_chain->to_chain)) { if(chain_exists(opts, IPT_OUTPUT_ACCESS) == 0) create_chain(opts, IPT_OUTPUT_ACCESS); @@ -786,7 +793,7 @@ process_spa_request(const fko_srv_options_t * const opts, /* If we have to make an corresponding OUTPUT rule if out_chain target * is not NULL. */ - if(out_chain->to_chain != NULL && strlen(out_chain->to_chain)) + if(strlen(out_chain->to_chain)) { memset(rule_buf, 0, CMD_BUFSIZE); @@ -884,7 +891,7 @@ process_spa_request(const fko_srv_options_t * const opts, } } - else if(fwd_chain->to_chain != NULL && strlen(fwd_chain->to_chain)) + else if(strlen(fwd_chain->to_chain)) { /* Make our FORWARD and NAT rules, and make sure the * required chain and jump rule exists @@ -927,7 +934,7 @@ process_spa_request(const fko_srv_options_t * const opts, } } - if(dnat_chain->to_chain != NULL && strlen(dnat_chain->to_chain)) + if(strlen(dnat_chain->to_chain)) { /* Make sure the required chain and jump rule exists */ From d60870740da90c2eca0a8910dd5cd616438ddabd Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Mon, 13 May 2013 20:41:25 -0400 Subject: [PATCH 3/7] [server] fix pointer NULL check after strdup() - found by Coverity --- server/incoming_spa.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/incoming_spa.c b/server/incoming_spa.c index 81e3722a..c352e3d9 100644 --- a/server/incoming_spa.c +++ b/server/incoming_spa.c @@ -188,8 +188,8 @@ get_raw_digest(char **digest, char *pkt_data) *digest = strdup(tmp_digest); - if (digest == NULL) - return SPA_MSG_ERROR; + if (*digest == NULL) + return SPA_MSG_ERROR; /* really a strdup() memory allocation problem */ fko_destroy(ctx); ctx = NULL; From 8e31f8feb02585e1b110efd6e01228425bff11ce Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Mon, 13 May 2013 20:42:07 -0400 Subject: [PATCH 4/7] [server] varargs cleanup bug fix found by Coverity --- server/log_msg.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/log_msg.c b/server/log_msg.c index 0f3861f9..48d6950b 100644 --- a/server/log_msg.c +++ b/server/log_msg.c @@ -164,7 +164,10 @@ log_msg(int level, char* msg, ...) va_end(apse); if(LOG_STDERR_ONLY & level) + { + va_end(ap); return; + } /* Remove the log to stderr flag from the log level value. */ From 6a2bc3db2718ab06c07c93b208dbd072d0ba5560 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Mon, 13 May 2013 20:48:23 -0400 Subject: [PATCH 5/7] [server] minor memory leak bug fix during access.conf parsing found by Coverity --- server/access.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/access.c b/server/access.c index 88ca7fd8..88039c9c 100644 --- a/server/access.c +++ b/server/access.c @@ -1365,13 +1365,15 @@ acc_check_port_access(acc_stanza_t *acc, char *port_str) "Unable to create acc_port_list from incoming data: %s", port_str ); + free_acc_port_list(in_pl); return(0); } strlcpy(buf, start, (ndx-start)+1); if(add_port_list_ent(&in_pl, buf) == 0) { log_msg(LOG_ERR, "Invalid proto/port string"); - return 0; + free_acc_port_list(in_pl); + return(0); } start = ndx+1; @@ -1386,6 +1388,7 @@ acc_check_port_access(acc_stanza_t *acc, char *port_str) "Unable to create acc_port_list from incoming data: %s", port_str ); + free_acc_port_list(in_pl); return(0); } strlcpy(buf, start, (ndx-start)+1); From fb80575209a8276767457b2c5fefaa42ea1aca23 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Mon, 13 May 2013 20:52:14 -0400 Subject: [PATCH 6/7] [server] minor memory leak bug fix during SPA digest calculation found by Coverity --- server/incoming_spa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/incoming_spa.c b/server/incoming_spa.c index c352e3d9..2403360a 100644 --- a/server/incoming_spa.c +++ b/server/incoming_spa.c @@ -189,7 +189,7 @@ get_raw_digest(char **digest, char *pkt_data) *digest = strdup(tmp_digest); if (*digest == NULL) - return SPA_MSG_ERROR; /* really a strdup() memory allocation problem */ + res = SPA_MSG_ERROR; /* really a strdup() memory allocation problem */ fko_destroy(ctx); ctx = NULL; From e73d13e14086b00435f0248d8d8a7df0885a771f Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Mon, 13 May 2013 23:11:33 -0400 Subject: [PATCH 7/7] minor write_test_file() path bug fix --- test/test-fwknop.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-fwknop.pl b/test/test-fwknop.pl index 0aad5b9c..384ffa29 100755 --- a/test/test-fwknop.pl +++ b/test/test-fwknop.pl @@ -5690,7 +5690,7 @@ sub file_find_regex() { } open F, "< $file" or - (&write_test_file("[-] Could not open $file: $!\n", $file) and return 0); + (&write_test_file("[-] Could not open $file: $!\n", $curr_test_file) and return 0); while () { push @file_lines, $_; }