From 21149faf8914dc1721ca1ec7aacacae496676cc3 Mon Sep 17 00:00:00 2001
From: Michael Rash <mbr@cipherdyne.org>
Date: Fri, 4 Dec 2015 19:01:26 -0800
Subject: [PATCH] [libfko] fko_set_username() crash bug fix.

Bug fix for a crash in libfko that could be triggered in fko_set_username()
when a username that is 64 chars or longer is specified. This crash
cannot be triggered in fwknopd even if an SPA packet contains such a
username however due to additional protections in the SPA decoding
routines. Further, this bug does not apply to the main fwknop client
either because the maximal username size is truncated down below 64
bytes. Hence, this bug only applies to client-side software that is
directly using libfko calling the fko_set_username() function.
---
 lib/fko_user.c                         | 16 +++++++++------
 test/fko-wrapper/fko_fault_injection.c |  6 +++++-
 test/tests/basic_operations.pl         | 27 ++++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/lib/fko_user.c b/lib/fko_user.c
index ca6d5460..1489c478 100644
--- a/lib/fko_user.c
+++ b/lib/fko_user.c
@@ -55,8 +55,16 @@ fko_set_username(fko_ctx_t ctx, const char * const spoof_user)
     /* If spoof_user was not passed in, check for a SPOOF_USER enviroment
      * variable.  If it is set, use its value.
     */
-    if(spoof_user != NULL && strnlen(spoof_user, MAX_SPA_USERNAME_SIZE))
-        username = (char*)spoof_user;
+    if(spoof_user != NULL && spoof_user[0] != '\0')
+    {
+#if HAVE_LIBFIU
+        fiu_return_on("fko_set_username_strdup", FKO_ERROR_MEMORY_ALLOCATION);
+#endif
+        username = strdup(spoof_user);
+        if(username == NULL)
+            return(FKO_ERROR_MEMORY_ALLOCATION);
+        is_user_heap_allocated = 1;
+    }
     else
         username = getenv("SPOOF_USER");
 
@@ -112,10 +120,6 @@ fko_set_username(fko_ctx_t ctx, const char * const spoof_user)
     if(ctx->username != NULL)
         free(ctx->username);
 
-#if HAVE_LIBFIU
-    fiu_return_on("fko_set_username_strdup", FKO_ERROR_MEMORY_ALLOCATION);
-#endif
-
     ctx->username = strdup(username);
 
     ctx->state |= FKO_DATA_MODIFIED;
diff --git a/test/fko-wrapper/fko_fault_injection.c b/test/fko-wrapper/fko_fault_injection.c
index 5071ece2..96bccf02 100644
--- a/test/fko-wrapper/fko_fault_injection.c
+++ b/test/fko-wrapper/fko_fault_injection.c
@@ -77,7 +77,11 @@ int main(void) {
                     strlen("fko_set_username_valuser")) == 0)
             res = fko_set_username(ctx, "BADCHAR=");
 
-        if (res == FKO_SUCCESS)
+        if(strncmp(fiu_tags[i], "fko_set_username_strdup",
+                    strlen("fko_set_username_strdup")) == 0)
+            res = fko_set_username(ctx, "normaluser");
+
+        if(res == FKO_SUCCESS)
         {
             printf("[-] fko_new(): %s\n", fko_errstr(res));
             fail++;
diff --git a/test/tests/basic_operations.pl b/test/tests/basic_operations.pl
index 837e8c7f..06ee0711 100644
--- a/test/tests/basic_operations.pl
+++ b/test/tests/basic_operations.pl
@@ -193,6 +193,20 @@
         'exec_err' => $YES,
         'cmdline' => "$fwknopCmd -A tcp/600001 -a $fake_ip -D $loopback_ip",
     },
+    {
+        'category' => 'basic operations',
+        'subcategory' => 'client',
+        'detail'   => '--spoof-user (long user)',
+        'function' => \&generic_exec,
+        'cmdline'  => "$default_client_hmac_args --spoof-user " . 'A'x80
+    },
+    {
+        'category' => 'basic operations',
+        'subcategory' => 'client',
+        'detail'   => 'env SPOOF_USER (long user)',
+        'function' => \&generic_exec,
+        'cmdline'  => "SPOOF_USER=" . 'A'x80 . ' ' . $default_client_hmac_args
+    },
 
     {
         'category' => 'basic operations',
@@ -1088,6 +1102,19 @@
         'exec_err' => $YES,
         'positive_output_matches' => [qr/Args\scontain\sinvalid/],
     },
+    {
+        'category' => 'basic operations',
+        'subcategory' => 'client save rc file',
+        'detail'   => '--spoof-user (long user)',
+        'function' => \&client_rc_file,
+        'cmdline'  => "$client_save_rc_args -n default " .
+                "--spoof-user " . 'A'x80,
+        'save_rc_stanza' => [{'name' => 'default',
+                'vars' => {'KEY' => 'testtest', 'HMAC_KEY' => 'hmactest',
+                    'HMAC_DIGEST_TYPE' => 'SHA1'}}],
+        'positive_output_matches' => [qr/Username\:\sAAAA/],
+        'rc_positive_output_matches' => [qr/SPOOF_USER.*AAAA/],
+    },
 
     {
         'category' => 'basic operations',