[Openvpn-devel,v2] crypto_openssl.c: fix heap-buffer-overflow found by AddressSanitizer

Message ID 1548164463-13366-1-git-send-email-lstipakov@gmail.com
State Accepted
Headers show
Series
  • [Openvpn-devel,v2] crypto_openssl.c: fix heap-buffer-overflow found by AddressSanitizer
Related show

Commit Message

Lev Stipakov Jan. 22, 2019, 1:41 p.m.
From: Lev Stipakov <lev@openvpn.net>

OpenSSL's version of crypto_pem_encode() uses PEM_write_bio()
function to write PEM-encoded data to BIO object. That method doesn't
add NUL termanator, unlike its mbedTLS counterpart mbedtls_pem_write_buffer().

The code which uses PEM data treats it as a string, so missing NUL
terminator makes sanitizer to compain.

Fix by adding a NUL terminator.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
---
 v2: use a dedivcated function to add a nul terminator

 src/openvpn/crypto_openssl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Arne Schwabe Jan. 22, 2019, 1:45 p.m. | #1
Am 22.01.19 um 14:41 schrieb Lev Stipakov:
> From: Lev Stipakov <lev@openvpn.net>
> 
> OpenSSL's version of crypto_pem_encode() uses PEM_write_bio()
> function to write PEM-encoded data to BIO object. That method doesn't
> add NUL termanator, unlike its mbedTLS counterpart mbedtls_pem_write_buffer().
> 
> The code which uses PEM data treats it as a string, so missing NUL
> terminator makes sanitizer to compain.
> 
> Fix by adding a NUL terminator.
> 
> Signed-off-by: Lev Stipakov <lev@openvpn.net>
> ---
>  v2: use a dedivcated function to add a nul terminator
                 ^^^ :)

Acked-By: Arne Schwabe

Arne
Gert Doering Jan. 22, 2019, 4:20 p.m. | #2
Your patch has been applied to the master branch.

commit eb1fed3f3bb817332183672dd1ca665ece83d6a8
Author: Lev Stipakov
Date:   Tue Jan 22 15:41:03 2019 +0200

     crypto_openssl.c: fix heap-buffer-overflow found by AddressSanitizer

     Signed-off-by: Lev Stipakov <lev@openvpn.net>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <1548164463-13366-1-git-send-email-lstipakov@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18141.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 9691ce0..c049e52 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -400,8 +400,9 @@  crypto_pem_encode(const char *name, struct buffer *dst,
     BUF_MEM *bptr;
     BIO_get_mem_ptr(bio, &bptr);
 
-    *dst = alloc_buf_gc(bptr->length, gc);
+    *dst = alloc_buf_gc(bptr->length + 1, gc);
     ASSERT(buf_write(dst, bptr->data, bptr->length));
+    buf_null_terminate(dst);
 
     ret = true;
 cleanup: