[Openvpn-devel,v1] Fix "uninitialized pointer read" in openvpn_decrypt_aead

Message ID 20250113112226.17728-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel,v1] Fix "uninitialized pointer read" in openvpn_decrypt_aead | expand

Commit Message

Gert Doering Jan. 13, 2025, 11:22 a.m. UTC
From: Frank Lichtenheld <frank@lichtenheld.com>

Coverity complains that if we error out in the first
error condition we try to free gc without initializing
it.

While here move the declaration of outlen to the first
usage.

Change-Id: I0391f30a1e962ee242e9bcdec4f605bf7e831cca
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Antonio Quartulli <a@unstable.cc>
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/858
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Antonio Quartulli <a@unstable.cc>

Comments

Gert Doering Jan. 13, 2025, 11:43 a.m. UTC | #1
"Obviously correct" :-) - not tested further (Coverity found this and
will tell us if it considers the code fixed now).

Note that this has a very limited impact - only if running very recent
master, and only if receiving more than 2^36 packets that can not
be decrypted - then we might crash due to free()ing an uninitialized
pointer.

Your patch has been applied to the master branch.

commit 5e086c08f2ce4428fd014b74441f0197a71d6da8
Author: Frank Lichtenheld
Date:   Mon Jan 13 12:22:26 2025 +0100

     Fix 'uninitialized pointer read' in openvpn_decrypt_aead

     Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
     Acked-by: Antonio Quartulli <a@unstable.cc>
     Message-Id: <20250113112226.17728-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30421.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 84ec436..dbd95a8 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -406,17 +406,15 @@ 
     static const char error_prefix[] = "AEAD Decrypt error";
     struct packet_id_net pin = { 0 };
     struct key_ctx *ctx = &opt->key_ctx_bi.decrypt;
+    struct gc_arena gc;
+
+    gc_init(&gc);
 
     if (cipher_decrypt_verify_fail_exceeded(ctx))
     {
         CRYPT_DROP("Decryption failed verification limit reached.");
     }
 
-    int outlen;
-    struct gc_arena gc;
-
-    gc_init(&gc);
-
     ASSERT(opt);
     ASSERT(frame);
     ASSERT(buf->len > 0);
@@ -506,6 +504,8 @@ 
     dmsg(D_PACKET_CONTENT, "DECRYPT AD: %s",
          format_hex(ad_start, ad_size, 0, &gc));
 
+    int outlen;
+
     /* Decrypt and authenticate packet */
     if (!cipher_ctx_update(ctx->cipher, BPTR(&work), &outlen, BPTR(buf),
                            data_len))