From: Steffan Karger Date: Fri, 1 Apr 2016 16:43:00 +0000 (+0200) Subject: Fix potential null-pointer dereference X-Git-Tag: v2.4_alpha1~120 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b064b8111c718f3c4f996f256674ccd3ab62217f;p=thirdparty%2Fopenvpn.git Fix potential null-pointer dereference Commit a070f75b (master branch only) changed the openvpn_encrypt logic and now prepends the contents of the work buffer to buf if no encryption is used (which is the case for tls-auth packets). In that case, the code would potentially dereference a null-pointer in a memcpy(some-dest, 0, 0) call. Fortunately, memcpy() inplementations usually do not actually derefence the src (or dst) pointer for zero-length copies. And since I'm touching this code now anyway, remove a slightly confusing jump back to a cleanup label in openvpn_encrypt_aead(). Issue spotted by Daniel Hirche. Signed-off-by: Steffan Karger Acked-by: Gert Doering Message-Id: <1459528980-8304-1-git-send-email-steffan@karger.me> URL: http://article.gmane.org/gmane.network.openvpn.devel/11372 Signed-off-by: Gert Doering --- diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index f15ac3511..5c392aad3 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -166,14 +166,14 @@ openvpn_encrypt_aead (struct buffer *buf, struct buffer work, dmsg (D_PACKET_CONTENT, "ENCRYPT TO: %s", format_hex (BPTR (buf), BLEN (buf), 80, &gc)); -cleanup: gc_free (&gc); return; err: crypto_clear_error(); buf->len = 0; - goto cleanup; + gc_free (&gc); + return; #else /* HAVE_AEAD_CIPHER_MODES */ ASSERT (0); #endif @@ -295,7 +295,9 @@ openvpn_encrypt_v1 (struct buffer *buf, struct buffer work, hmac_start = BPTR(buf); ASSERT (mac_out = buf_prepend (buf, hmac_ctx_size(ctx->hmac))); } - buf_write_prepend(buf, BPTR(&work), BLEN(&work)); + if (BLEN(&work)) { + buf_write_prepend(buf, BPTR(&work), BLEN(&work)); + } work = *buf; }