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().

---
 src/openvpn/crypto.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

index f15ac35..5c392aa 100644
@@ -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;
 	}
 

2.5.0

ACK - "because it obviously makes sense" :-) - thanks.

Your patch has been applied to the master branch.

I have added a note to credit the bug spotter (Daniel Hirche).

commit b064b8111c718f3c4f996f256674ccd3ab62217f

Author: Steffan Karger
Date:   Fri Apr 1 18:43:00 2016 +0200

     Fix potential null-pointer dereference

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/11372
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering