Fix buffer overflow vulnerability in spa_base64_to_bits() function.

4 files changed, 35 insertions, 12 deletions

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index eff7a9d7e..ba8b15bfb 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -1,4 +1,4 @@
-$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.58 2004/12/29 10:16:52 ph10 Exp $
+$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.59 2004/12/29 10:55:58 ph10 Exp $
 
 Change log file for Exim from version 4.21
 -------------------------------------------
@@ -257,9 +257,20 @@ Exim version 4.50
     to be a valid IP address. However, in the case of IPv6 addresses, it was
     not checking this. This is a hostage to fortune. Exim now panics and dies
     if the condition is not met. A case was found where this could be provoked
-    from a dnsdb lookup; fortuitously, this particular loophole had already
-    been fixed by change 4.50/55 above. If there are any other similar
-    loopholes, the new check should stop them being exploited.
+    from a dnsdb PTR lookup with an IPv6 address that had more than 8
+    components; fortuitously, this particular loophole had already been fixed
+    by change 4.50/55 above.
+
+    If there are any other similar loopholes, the new check in host_aton()
+    itself should stop them being exploited. The report I received stated that
+    data on the command line could provoke the exploit when Exim was running as
+    exim, but did not say which command line option was involved. All I could
+    find was the use of -be with a bad dnsdb PTR lookup, and in that case it is
+    running as the user.
+
+61. There was a buffer overflow vulnerability in the SPA authentication code
+    (which came originally from the Samba project). I have added a test to the
+    spa_base64_to_bits() function which I hope fixes it.
 
 
 Exim version 4.43
diff --git a/src/src/auths/auth-spa.c b/src/src/auths/auth-spa.c
index c6f716551..bd7fd41ed 100644
--- a/src/src/auths/auth-spa.c
+++ b/src/src/auths/auth-spa.c
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/src/src/auths/auth-spa.c,v 1.1 2004/10/07 13:10:00 ph10 Exp $ */
+/* $Cambridge: exim/src/src/auths/auth-spa.c,v 1.2 2004/12/29 10:55:58 ph10 Exp $ */
 
 /*************************************************
 *     Exim - an Internet mail transport agent    *
@@ -406,8 +406,11 @@ spa_bits_to_base64 (unsigned char *out, const unsigned char *in, int inlen)
   *out = '\0';
 }
 
+
+/* The outlength parameter was added by PH, December 2004 */
+
 int
-spa_base64_to_bits (char *out, const char *in)
+spa_base64_to_bits (char *out, int outlength, const char *in)
 /* base 64 to raw bytes in quasi-big-endian order, returning count of bytes */
 {
   int len = 0;
@@ -420,6 +423,8 @@ spa_base64_to_bits (char *out, const char *in)
 
   do
     {
+      if (len >= outlength)                   /* Added by PH */
+        return (-1);                          /* Added by PH */
       digit1 = in[0];
       if (DECODE64 (digit1) == BAD)
        return (-1);
@@ -437,11 +442,15 @@ spa_base64_to_bits (char *out, const char *in)
       ++len;
       if (digit3 != '=')
        {
+         if (len >= outlength)                   /* Added by PH */
+           return (-1);                          /* Added by PH */
          *out++ =
            ((DECODE64 (digit2) << 4) & 0xf0) | (DECODE64 (digit3) >> 2);
          ++len;
          if (digit4 != '=')
            {
+             if (len >= outlength)                   /* Added by PH */
+               return (-1);                          /* Added by PH */
              *out++ = ((DECODE64 (digit3) << 6) & 0xc0) | DECODE64 (digit4);
              ++len;
            }
diff --git a/src/src/auths/auth-spa.h b/src/src/auths/auth-spa.h
index 52394e570..fb4e22ac1 100644
--- a/src/src/auths/auth-spa.h
+++ b/src/src/auths/auth-spa.h
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/src/src/auths/auth-spa.h,v 1.1 2004/10/07 13:10:00 ph10 Exp $ */
+/* $Cambridge: exim/src/src/auths/auth-spa.h,v 1.2 2004/12/29 10:55:58 ph10 Exp $ */
 
 /*************************************************
 *     Exim - an Internet mail transport agent    *
@@ -11,6 +11,9 @@
  * All the code used here was torn by Marc Prud'hommeaux out of the
  * Samba project (by Andrew Tridgell, Jeremy Allison, and others).
  */
+ 
+/* December 2004: The spa_base64_to_bits() function has no length checking in 
+it. I have added a check. PH */ 
 
 /* It seems that some systems have existing but different definitions of some
 of the following types. I received a complaint about "int16" causing
@@ -77,7 +80,7 @@ typedef struct
 #define spa_request_length(ptr) (((ptr)->buffer - (uint8x*)(ptr)) + (ptr)->bufIndex)
 
 void spa_bits_to_base64 (unsigned char *, const unsigned char *, int);
-int spa_base64_to_bits(char *, const char *);
+int spa_base64_to_bits(char *, int, const char *);
 void spa_build_auth_response (SPAAuthChallenge *challenge,
        SPAAuthResponse *response, char *user, char *password);
 void spa_build_auth_request (SPAAuthRequest *request, char *user,
diff --git a/src/src/auths/spa.c b/src/src/auths/spa.c
index dc859674e..0f53152b4 100644
--- a/src/src/auths/spa.c
+++ b/src/src/auths/spa.c
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/src/src/auths/spa.c,v 1.2 2004/12/20 14:57:05 ph10 Exp $ */
+/* $Cambridge: exim/src/src/auths/spa.c,v 1.3 2004/12/29 10:55:58 ph10 Exp $ */
 
 /*************************************************
 *     Exim - an Internet mail transport agent    *
@@ -135,7 +135,7 @@ if (auth_get_no64_data(&data, US"NTLM supported") != OK)
   return FAIL;
   }
 
-if (spa_base64_to_bits((char *)(&request), (const char *)(data)) < 0)
+if (spa_base64_to_bits((char *)(&request), sizeof(request), (const char *)(data)) < 0)
   {
   DEBUG(D_auth) debug_printf("auth_spa_server(): bad base64 data in "
   "request: %s\n", data);
@@ -155,7 +155,7 @@ if (auth_get_no64_data(&data, msgbuf) != OK)
   }
 
 /* dump client response */
-if (spa_base64_to_bits((char *)(&response), (const char *)(data)) < 0)
+if (spa_base64_to_bits((char *)(&response), sizeof(response), (const char *)(data)) < 0)
   {
   DEBUG(D_auth) debug_printf("auth_spa_server(): bad base64 data in "
   "response: %s\n", data);
@@ -324,7 +324,7 @@ auth_spa_client(
        /* convert the challenge into the challenge struct */
        DSPA("\n\n%s authenticator: challenge (%s)\n\n",
                ablock->name, buffer + 4);
-       spa_base64_to_bits ((char *)(&challenge), (const char *)(buffer + 4));
+       spa_base64_to_bits ((char *)(&challenge), sizeof(challenge), (const char *)(buffer + 4));
 
        spa_build_auth_response (&challenge, &response,
                CS username, CS password);