From 41f12eba81869814ceb6f5c809e90442d6a56b40 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Wed, 26 Mar 2014 21:14:11 -0400 Subject: [PATCH] [libfko] Memory leak bug fix in GnuPG handling Bug fix to correct a memory leak in GnuPG SPA packet handling within the gpg_decrypt() function. Here is the specific valgrind leak record that enabled the bug to be found (note that the new valgrind suppressions usage was critical for finding this bug among all other libgpgme memory leaks): ==23983== 1,044 bytes in 1 blocks are definitely lost in loss record 7 of 8 ==23983== at 0x4C2C494: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==23983== by 0x4E41D3A: gpg_decrypt (fko_encryption.c:422) ==23983== by 0x4E42520: fko_decrypt_spa_data (fko_encryption.c:626) ==23983== by 0x1155B0: incoming_spa (incoming_spa.c:519) ==23983== by 0x1180A7: process_packet (process_packet.c:211) ==23983== by 0x506D857: ??? (in /usr/lib/x86_64-linux-gnu/libpcap.so.1.4.0) ==23983== by 0x117865: pcap_capture (pcap_capture.c:270) ==23983== by 0x10F937: main (fwknopd.c:353) --- ChangeLog | 20 ++++++++++++++++++++ lib/fko_encryption.c | 10 ++++------ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index db22396a..4f3efd7d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -10,6 +10,24 @@ fwknop-2.6.1 (//2014): signature. Verification of GnuPG signatures can be disabled with a new access.conf variable GPG_DISABLE_SIG, but this is NOT a recommended configuration. + - [libfko] Bug fix to correct a memory leak in GnuPG SPA packet handling + within the gpg_decrypt() function. Here is the specific valgrind leak + record that enabled the bug to be found (note that the new valgrind + suppressions usage was critical for finding this bug among all other + libgpgme memory leaks): + + ==23983== 1,044 bytes in 1 blocks are definitely lost in loss record 7 of 8 + ==23983== at 0x4C2C494: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) + ==23983== by 0x4E41D3A: gpg_decrypt (fko_encryption.c:422) + ==23983== by 0x4E42520: fko_decrypt_spa_data (fko_encryption.c:626) + ==23983== by 0x1155B0: incoming_spa (incoming_spa.c:519) + ==23983== by 0x1180A7: process_packet (process_packet.c:211) + ==23983== by 0x506D857: ??? (in /usr/lib/x86_64-linux-gnu/libpcap.so.1.4.0) + ==23983== by 0x117865: pcap_capture (pcap_capture.c:270) + ==23983== by 0x10F937: main (fwknopd.c:353) + + - [libfko] Better SPA packet dumping output to include GnuPG specifics + whenever SPA packets are encrypted/authenticated via GnuPG. - [client+server] Add --gpg-exe command line argument and GPG_EXE config variable to ~/.fwknoprc and the access.conf file so that the path to GnuPG can be changed from the default /usr/bin/gpg path. @@ -22,6 +40,8 @@ fwknop-2.6.1 (//2014): - [client] Minor bug fixes in 'udpraw' mode (normally used for spoofing the SPA source IP) to set a non-zero source port and to properly set the length field of the UDP header. + - [test suite] Added valgrind suppressions for gpgme library function + calls. - [test suite] Added Rijndael+HMAC command execution test. - [test suite] Added Rijndael+HMAC NAT rand port via client rc file test. diff --git a/lib/fko_encryption.c b/lib/fko_encryption.c index f71a9a16..dad47a6f 100644 --- a/lib/fko_encryption.c +++ b/lib/fko_encryption.c @@ -450,13 +450,11 @@ gpg_decrypt(fko_ctx_t ctx, const char *dec_key) /* Done with cipher... */ - if(res != FKO_SUCCESS) - { - if(zero_free((char *) cipher, ctx->encrypted_msg_len) != FKO_SUCCESS) - return(FKO_ERROR_ZERO_OUT_DATA); - else + if(zero_free((char *) cipher, ctx->encrypted_msg_len) != FKO_SUCCESS) + return(FKO_ERROR_ZERO_OUT_DATA); + else + if(res != FKO_SUCCESS) /* bail if there was some other problem */ return(res); - } pt_len = strnlen(ctx->encoded_msg, MAX_SPA_ENCODED_MSG_SIZE);