From ac38f8d9938146775bb336f5a8b7680492b81102 Mon Sep 17 00:00:00 2001
From: Michael Rash <mbr@cipherdyne.org>
Date: Fri, 26 Oct 2012 15:36:08 -0400
Subject: [PATCH] [libfko] bug fix to check b64_decode() return value

Bug fix to check b64_decode() return value to ensure that
non-base64 encoded data is never used.  Even though other validation
routines checked decoded results, it is important to discard invalid
data as early as possible.  Note too that such invalid data would only
be provided to b64_decode() after proper decryption, so the client must
provide authentic SPA data.
---
 ChangeLog            |  8 +++++++-
 lib/fko_decode.c     | 30 +++++++++++++++++++++++++-----
 lib/fko_encryption.c |  6 ++++--
 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 87efa35c..2e33e2c3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,4 @@
-fwknop-2.0.4 (09/20/2012):
+fwknop-2.0.4 (11//2012):
     - [server] Added upstart config at extras/upstart/fwknop.conf.  This
       allows the fwknopd to easily be managed with upstart via commands like
       "service fwknop start" and "service fwknop stop".
@@ -17,6 +17,12 @@ fwknop-2.0.4 (09/20/2012):
       UDP with a spoofed source IP address.  This is in addition to the
       original 'tcpraw' and 'icmp' protocols that also support a spoofed
       source IP.
+    - [libfko] Bug fix to check b64_decode() return value to ensure that
+      non-base64 encoded data is never used.  Even though other validation
+      routines checked decoded results, it is important to discard invalid
+      data as early as possible.  Note too that such invalid data would only
+      be provided to b64_decode() after proper decryption, so the client must
+      provide authentic SPA data.
     - [libfko] Added validation of NAT access strings in the various NAT
       modes.
     - [libfko] Restricted usernames embedded in SPA packets to be
diff --git a/lib/fko_decode.c b/lib/fko_decode.c
index 22ac92b4..2706763d 100644
--- a/lib/fko_decode.c
+++ b/lib/fko_decode.c
@@ -195,7 +195,11 @@ fko_decode_spa_data(fko_ctx_t ctx)
         return(FKO_ERROR_MEMORY_ALLOCATION);
     }
 
-    b64_decode(tbuf, (unsigned char*)ctx->username);
+    if(b64_decode(tbuf, (unsigned char*)ctx->username) < 0)
+    {
+        free(tbuf);
+        return(FKO_ERROR_INVALID_DATA);
+    }
     if(validate_username(ctx->username) != FKO_SUCCESS)
     {
         free(tbuf);
@@ -294,7 +298,11 @@ fko_decode_spa_data(fko_ctx_t ctx)
         return(FKO_ERROR_MEMORY_ALLOCATION);
     }
 
-    b64_decode(tbuf, (unsigned char*)ctx->message);
+    if(b64_decode(tbuf, (unsigned char*)ctx->message) < 0)
+    {
+        free(tbuf);
+        return(FKO_ERROR_INVALID_DATA);
+    }
 
     if(ctx->message_type == FKO_COMMAND_MSG)
     {
@@ -346,7 +354,11 @@ fko_decode_spa_data(fko_ctx_t ctx)
             return(FKO_ERROR_MEMORY_ALLOCATION);
         }
 
-        b64_decode(tbuf, (unsigned char*)ctx->nat_access);
+        if(b64_decode(tbuf, (unsigned char*)ctx->nat_access) < 0)
+        {
+            free(tbuf);
+            return(FKO_ERROR_INVALID_DATA);
+        }
 
         if(validate_nat_access_msg(ctx->nat_access) != FKO_SUCCESS)
         {
@@ -383,7 +395,11 @@ fko_decode_spa_data(fko_ctx_t ctx)
                 return(FKO_ERROR_MEMORY_ALLOCATION);
             }
 
-            b64_decode(tbuf, (unsigned char*)ctx->server_auth);
+            if(b64_decode(tbuf, (unsigned char*)ctx->server_auth) < 0)
+            {
+                free(tbuf);
+                return(FKO_ERROR_INVALID_DATA);
+            }
 
             /* At this point we should be done.
             */
@@ -423,7 +439,11 @@ fko_decode_spa_data(fko_ctx_t ctx)
                 return(FKO_ERROR_MEMORY_ALLOCATION);
             }
 
-            b64_decode(tbuf, (unsigned char*)ctx->server_auth);
+            if(b64_decode(tbuf, (unsigned char*)ctx->server_auth) < 0)
+            {
+                free(tbuf);
+                return(FKO_ERROR_INVALID_DATA);
+            }
 
             ndx += t_size + 1;
         }
diff --git a/lib/fko_encryption.c b/lib/fko_encryption.c
index fb742dca..e366f7ad 100644
--- a/lib/fko_encryption.c
+++ b/lib/fko_encryption.c
@@ -135,7 +135,8 @@ _rijndael_decrypt(fko_ctx_t ctx, const char *dec_key)
     if(cipher == NULL)
         return(FKO_ERROR_MEMORY_ALLOCATION);
 
-    cipher_len = b64_decode(ctx->encrypted_msg, cipher);
+    if((cipher_len = b64_decode(ctx->encrypted_msg, cipher)) < 0)
+        return(FKO_ERROR_INVALID_DATA);
 
     /* Create a bucket for the plaintext data and decrypt the message
      * data into it.
@@ -285,7 +286,8 @@ gpg_decrypt(fko_ctx_t ctx, const char *dec_key)
     if(cipher == NULL)
         return(FKO_ERROR_MEMORY_ALLOCATION);
 
-    cipher_len = b64_decode(ctx->encrypted_msg, cipher);
+    if((cipher_len = b64_decode(ctx->encrypted_msg, cipher)) < 0)
+        return(FKO_ERROR_INVALID_DATA);
 
     /* Create a bucket for the plaintext data and decrypt the message
      * data into it.