[Openvpn-devel,2/2] test_tls_crypt.c: fix global-buffer-overflow found by AddressSanitizer

Message ID 1548154968-13019-2-git-send-email-lstipakov@gmail.com
State Superseded
Headers show
Series
  • [Openvpn-devel,1/2] crypto_openssl.c: fix heap-buffer-overflow found by AddressSanitizer
Related show

Commit Message

Lev Stipakov Jan. 22, 2019, 11:02 a.m.
From: Lev Stipakov <lev@openvpn.net>

When writing data to buffer we incorrectly specify source length
 - sizeof for pointer returns 8, but actual buffer length is 1.

Fix by replacing empty global string to local string literal and
specifying the correct length.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
---
 tests/unit_tests/openvpn/test_tls_crypt.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Arne Schwabe Jan. 22, 2019, 12:36 p.m. | #1
Am 22.01.19 um 12:02 schrieb Lev Stipakov:
> From: Lev Stipakov <lev@openvpn.net>
> 
> When writing data to buffer we incorrectly specify source length
>  - sizeof for pointer returns 8, but actual buffer length is 1.
> 
> Fix by replacing empty global string to local string literal and
> specifying the correct length.
> 
> Signed-off-by: Lev Stipakov <lev@openvpn.net>
> ---
>  tests/unit_tests/openvpn/test_tls_crypt.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c b/tests/unit_tests/openvpn/test_tls_crypt.c
> index b793a7a..b4ce03d 100644
> --- a/tests/unit_tests/openvpn/test_tls_crypt.c
> +++ b/tests/unit_tests/openvpn/test_tls_crypt.c
> @@ -49,8 +49,6 @@
>  #define PARAM1      "param1"
>  #define PARAM2      "param two"
>  
> -static const char *plaintext_short = "";
> -
>  static const char *test_server_key = \
>          "-----BEGIN OpenVPN tls-crypt-v2 server key-----\n"
>          "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4v\n"
> @@ -148,7 +146,7 @@ test_tls_crypt_setup(void **state) {
>      ctx->unwrapped = alloc_buf(TESTBUF_SIZE);
>  
>      /* Write test plaintext */
> -    buf_write(&ctx->source, plaintext_short, sizeof(plaintext_short));
> +    buf_write(&ctx->source, "1234567890", 10);
>  
>      /* Write dummy opcode and session id */
>      buf_write(&ctx->ciphertext, "012345678", 1 + 8);
> 

I would suggest plaintext_short="12345678"; and then use
strlen(plaintext_short)+1 in the buf_write.

But I can confirm that this indeed fixes the issue it should fix.

Arne

Patch

diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c b/tests/unit_tests/openvpn/test_tls_crypt.c
index b793a7a..b4ce03d 100644
--- a/tests/unit_tests/openvpn/test_tls_crypt.c
+++ b/tests/unit_tests/openvpn/test_tls_crypt.c
@@ -49,8 +49,6 @@ 
 #define PARAM1      "param1"
 #define PARAM2      "param two"
 
-static const char *plaintext_short = "";
-
 static const char *test_server_key = \
         "-----BEGIN OpenVPN tls-crypt-v2 server key-----\n"
         "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4v\n"
@@ -148,7 +146,7 @@  test_tls_crypt_setup(void **state) {
     ctx->unwrapped = alloc_buf(TESTBUF_SIZE);
 
     /* Write test plaintext */
-    buf_write(&ctx->source, plaintext_short, sizeof(plaintext_short));
+    buf_write(&ctx->source, "1234567890", 10);
 
     /* Write dummy opcode and session id */
     buf_write(&ctx->ciphertext, "012345678", 1 + 8);