From 9f9bbcbcdd8a47ee29bf60bb2f2728685bbc7aec Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Wed, 8 May 2013 23:55:35 -0400 Subject: [PATCH] fixed several resource leak conditions found by Coverity --- client/config_init.c | 9 +++++---- client/spa_comm.c | 1 + lib/fko_encryption.c | 23 ++++++++++++++++++++++- lib/fko_user.c | 10 +++++++++- server/fwknopd.c | 33 ++++++++++++++++++--------------- 5 files changed, 55 insertions(+), 21 deletions(-) diff --git a/client/config_init.c b/client/config_init.c index 0c9856f0..e13b2cc3 100644 --- a/client/config_init.c +++ b/client/config_init.c @@ -1148,10 +1148,10 @@ update_rc(fko_cli_options_t *options, uint32_t args_bitmask) rcfile_fd = open(rcfile_update, FWKNOPRC_OFLAGS, FWKNOPRC_MODE); if (rcfile_fd == -1) { - log_msg(LOG_VERBOSITY_WARNING, - "update_rc() : Unable to create temporary rc file: %s: %s", - rcfile_update, strerror(errno)); - return; + log_msg(LOG_VERBOSITY_WARNING, + "update_rc() : Unable to create temporary rc file: %s: %s", + rcfile_update, strerror(errno)); + return; } close(rcfile_fd); @@ -1170,6 +1170,7 @@ update_rc(fko_cli_options_t *options, uint32_t args_bitmask) log_msg(LOG_VERBOSITY_WARNING, "update_rc() : Unable to open rc file: %s: %s", rcfile_update, strerror(errno)); + fclose(rc); return; } diff --git a/client/spa_comm.c b/client/spa_comm.c index fea36794..2532669b 100644 --- a/client/spa_comm.c +++ b/client/spa_comm.c @@ -552,6 +552,7 @@ send_spa_packet_http(const char *spa_data, const int sd_len, log_msg(LOG_VERBOSITY_ERROR, "[-] proxy port value is invalid, must be in [%d-%d]", 1, MAX_PORT); + free(spa_data_copy); return 0; } } diff --git a/lib/fko_encryption.c b/lib/fko_encryption.c index f9dc5990..3acdd002 100644 --- a/lib/fko_encryption.c +++ b/lib/fko_encryption.c @@ -93,7 +93,10 @@ _rijndael_encrypt(fko_ctx_t ctx, const char *enc_key, const int enc_key_len) */ ciphertext = calloc(1, pt_len + 32); /* Plus padding for salt and Block */ if(ciphertext == NULL) + { + free(plaintext); return(FKO_ERROR_MEMORY_ALLOCATION); + } cipher_len = rij_encrypt( (unsigned char*)plaintext, pt_len, @@ -105,7 +108,11 @@ _rijndael_encrypt(fko_ctx_t ctx, const char *enc_key, const int enc_key_len) */ b64ciphertext = malloc(((cipher_len / 3) * 4) + 8); if(b64ciphertext == NULL) + { + free(ciphertext); + free(plaintext); return(FKO_ERROR_MEMORY_ALLOCATION); + } b64_encode(ciphertext, b64ciphertext, cipher_len); strip_b64_eq(b64ciphertext); @@ -158,7 +165,10 @@ _rijndael_decrypt(fko_ctx_t ctx, return(FKO_ERROR_MEMORY_ALLOCATION); if((cipher_len = b64_decode(ctx->encrypted_msg, cipher)) < 0) + { + free(cipher); return(FKO_ERROR_INVALID_DATA); + } /* Since we're using AES, make sure the incoming data is a multiple of * the blocksize @@ -177,7 +187,10 @@ _rijndael_decrypt(fko_ctx_t ctx, */ ctx->encoded_msg = malloc(cipher_len); if(ctx->encoded_msg == NULL) + { + free(cipher); return(FKO_ERROR_MEMORY_ALLOCATION); + } pt_len = rij_decrypt(cipher, cipher_len, dec_key, key_len, (unsigned char*)ctx->encoded_msg, encryption_mode); @@ -270,7 +283,10 @@ gpg_encrypt(fko_ctx_t ctx, const char *enc_key) pt_len = snprintf(plain, pt_len+1, "%s:%s", ctx->encoded_msg, ctx->digest); if(! is_valid_pt_msg_len(pt_len)) + { + free(plain); return(FKO_ERROR_INVALID_DATA); + } if (enc_key != NULL) { @@ -291,7 +307,7 @@ gpg_encrypt(fko_ctx_t ctx, const char *enc_key) { free(plain); - if(cipher) + if(cipher != NULL) free(cipher); return(res); @@ -301,7 +317,12 @@ gpg_encrypt(fko_ctx_t ctx, const char *enc_key) */ b64cipher = malloc(((cipher_len / 3) * 4) + 8); if(b64cipher == NULL) + { + free(plain); + if(cipher != NULL) + free(cipher); return(FKO_ERROR_MEMORY_ALLOCATION); + } b64_encode(cipher, b64cipher, cipher_len); strip_b64_eq(b64cipher); diff --git a/lib/fko_user.c b/lib/fko_user.c index 1f504d6e..c829902a 100644 --- a/lib/fko_user.c +++ b/lib/fko_user.c @@ -41,7 +41,7 @@ int fko_set_username(fko_ctx_t ctx, const char * const spoof_user) { char *username = NULL; - int res = FKO_SUCCESS; + int res = FKO_SUCCESS, is_user_heap_allocated=0; /* Must be initialized */ @@ -81,6 +81,7 @@ fko_set_username(fko_ctx_t ctx, const char * const spoof_user) username = strdup("NO_USER"); if(username == NULL) return(FKO_ERROR_MEMORY_ALLOCATION); + is_user_heap_allocated = 1; } } } @@ -92,7 +93,11 @@ fko_set_username(fko_ctx_t ctx, const char * const spoof_user) *(username + MAX_SPA_USERNAME_SIZE - 1) = '\0'; if((res = validate_username(username)) != FKO_SUCCESS) + { + if(is_user_heap_allocated == 1) + free(username); return res; + } /* Just in case this is a subsquent call to this function. We * do not want to be leaking memory. @@ -104,6 +109,9 @@ fko_set_username(fko_ctx_t ctx, const char * const spoof_user) ctx->state |= FKO_DATA_MODIFIED; + if(is_user_heap_allocated == 1) + free(username); + if(ctx->username == NULL) return(FKO_ERROR_MEMORY_ALLOCATION); diff --git a/server/fwknopd.c b/server/fwknopd.c index f3a1bf9b..53f85bcf 100644 --- a/server/fwknopd.c +++ b/server/fwknopd.c @@ -627,14 +627,14 @@ write_pid_file(fko_srv_options_t *opts) lck_res = lockf(op_fd, F_TLOCK, 0); if(lck_res == -1) { + close(op_fd); + if(errno != EAGAIN) { perror("Unexpected error from lockf: "); return -1; } - close(op_fd); - /* Look for an existing lock holder. If we get a pid return it. */ old_pid = get_running_pid(opts); @@ -685,22 +685,25 @@ get_running_pid(const fko_srv_options_t *opts) op_fd = open(opts->config[CONF_FWKNOP_PID_FILE], O_RDONLY); - if(op_fd > 0) + if(op_fd == -1) { - if (read(op_fd, buf, PID_BUFLEN) > 0) - { - buf[PID_BUFLEN-1] = '\0'; - /* max pid value is configurable on Linux - */ - rpid = (pid_t) strtol_wrapper(buf, 0, (2 << 30), - NO_EXIT_UPON_ERR, &is_err); - if(is_err != FKO_SUCCESS) - rpid = 0; - } - - close(op_fd); + perror("Error trying to open PID file: "); + return(rpid); } + if (read(op_fd, buf, PID_BUFLEN) > 0) + { + buf[PID_BUFLEN-1] = '\0'; + /* max pid value is configurable on Linux + */ + rpid = (pid_t) strtol_wrapper(buf, 0, (2 << 30), + NO_EXIT_UPON_ERR, &is_err); + if(is_err != FKO_SUCCESS) + rpid = 0; + } + + close(op_fd); + return(rpid); }