GitList

Browse code

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 <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
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 <gert@greenie.muc.de>

Steffan Karger authored on 2016/04/02 01:43:00
Showing 1 changed files

src/openvpn/crypto.c

History View file @ b064b81

@@ -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;
+                     	}