Fix SPA authenticator, checking client-supplied data before using it. Bug 2571

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 1d685a1308b0771722a34e0d0b00d23828c1104d..6109a14dd69433016ad56560fa90f54b79d4f2f4 100644 (file)
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -184,6 +184,11 @@ JH/40 Fix a memory-handling bug: when a connection carried multiple messages
       stale data could be accessed.  Ensure that variable references are
       dropped between messages.
 
       stale data could be accessed.  Ensure that variable references are
       dropped between messages.
 

+JH/41 Bug 2571: Fix SPA authenticator.  Running as a server, an offset supplied
+      by the client was not checked as pointing within response data before
+      being used.  A malicious client could thus cause an out-of-bounds read and
+      possibly gain authentication.  Fix by adding the check.
+

 
 Exim version 4.93
 -----------------
 
 Exim version 4.93
 -----------------

diff --git a/src/src/auths/auth-spa.c b/src/src/auths/auth-spa.c
index fc363df6fe5119826a21eea5244de5c25abe4842..44c99e9f6a68d2fc7cda8043a92f98a2bbe8511c 100644 (file)
--- a/src/src/auths/auth-spa.c
+++ b/src/src/auths/auth-spa.c
@@ -374,27 +374,27 @@ void
 spa_bits_to_base64 (uschar *out, const uschar *in, int inlen)
 /* raw bytes in quasi-big-endian order to base 64 string (NUL-terminated) */
 {
 spa_bits_to_base64 (uschar *out, const uschar *in, int inlen)
 /* raw bytes in quasi-big-endian order to base 64 string (NUL-terminated) */
 {

-  for (; inlen >= 3; inlen -= 3)
-    {
-      *out++ = base64digits[in[0] >> 2];
-      *out++ = base64digits[((in[0] << 4) & 0x30) | (in[1] >> 4)];
-      *out++ = base64digits[((in[1] << 2) & 0x3c) | (in[2] >> 6)];
-      *out++ = base64digits[in[2] & 0x3f];
-      in += 3;
-    }
-  if (inlen > 0)
-    {
-      uschar fragment;
-
-      *out++ = base64digits[in[0] >> 2];
-      fragment = (in[0] << 4) & 0x30;
-      if (inlen > 1)
-       fragment |= in[1] >> 4;
-      *out++ = base64digits[fragment];
-      *out++ = (inlen < 2) ? '=' : base64digits[(in[1] << 2) & 0x3c];
-      *out++ = '=';
-    }
-  *out = '\0';
+for (; inlen >= 3; inlen -= 3)
+  {
+  *out++ = base64digits[in[0] >> 2];
+  *out++ = base64digits[((in[0] << 4) & 0x30) | (in[1] >> 4)];
+  *out++ = base64digits[((in[1] << 2) & 0x3c) | (in[2] >> 6)];
+  *out++ = base64digits[in[2] & 0x3f];
+  in += 3;
+  }
+if (inlen > 0)
+  {
+  uschar fragment;
+
+  *out++ = base64digits[in[0] >> 2];
+  fragment = (in[0] << 4) & 0x30;
+  if (inlen > 1)
+     fragment |= in[1] >> 4;
+  *out++ = base64digits[fragment];
+  *out++ = (inlen < 2) ? '=' : base64digits[(in[1] << 2) & 0x3c];
+  *out++ = '=';
+  }
+*out = '\0';

 }
 
 
 }
 
 


@@ -404,52 +404,52 @@ int
 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 */
 {
 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;
-  register uschar digit1, digit2, digit3, digit4;
+int len = 0;
+uschar digit1, digit2, digit3, digit4;

 
 

-  if (in[0] == '+' && in[1] == ' ')
-    in += 2;
-  if (*in == '\r')
-    return (0);
+if (in[0] == '+' && in[1] == ' ')
+  in += 2;
+if (*in == '\r')
+  return (0);

 
 

-  do
+do
+  {
+  if (len >= outlength)                   /* Added by PH */
+    return -1;                          /* Added by PH */
+  digit1 = in[0];
+  if (DECODE64 (digit1) == BAD)
+    return -1;
+  digit2 = in[1];
+  if (DECODE64 (digit2) == BAD)
+    return -1;
+  digit3 = in[2];
+  if (digit3 != '=' && DECODE64 (digit3) == BAD)
+    return -1;
+  digit4 = in[3];
+  if (digit4 != '=' && DECODE64 (digit4) == BAD)
+    return -1;
+  in += 4;
+  *out++ = (DECODE64 (digit1) << 2) | (DECODE64 (digit2) >> 4);
+  ++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 */
       if (len >= outlength)                   /* Added by PH */

-        return (-1);                          /* Added by PH */
-      digit1 = in[0];
-      if (DECODE64 (digit1) == BAD)
-       return (-1);
-      digit2 = in[1];
-      if (DECODE64 (digit2) == BAD)
-       return (-1);
-      digit3 = in[2];
-      if (digit3 != '=' && DECODE64 (digit3) == BAD)
-       return (-1);
-      digit4 = in[3];
-      if (digit4 != '=' && DECODE64 (digit4) == BAD)
-       return (-1);
-      in += 4;
-      *out++ = (DECODE64 (digit1) << 2) | (DECODE64 (digit2) >> 4);
+       return -1;                          /* Added by PH */
+      *out++ = ((DECODE64 (digit3) << 6) & 0xc0) | DECODE64 (digit4);

       ++len;
       ++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;
-           }
-       }
+      }

     }
     }

-  while (*in && *in != '\r' && digit4 != '=');
+  }
+while (*in && *in != '\r' && digit4 != '=');

 
 

-  return (len);
+return len;

 }
 
 
 }
 
 

diff --git a/src/src/auths/spa.c b/src/src/auths/spa.c
index e7a588dd203e3afb9be8f4bdf5ac21d63e6fd810..f83d1144a75f08e415082034c06c136eb5205540 100644 (file)
--- a/src/src/auths/spa.c
+++ b/src/src/auths/spa.c
@@ -140,7 +140,7 @@ SPAAuthChallenge challenge;
 SPAAuthResponse  response;
 SPAAuthResponse  *responseptr = &response;
 uschar msgbuf[2048];
 SPAAuthResponse  response;
 SPAAuthResponse  *responseptr = &response;
 uschar msgbuf[2048];

-uschar *clearpass;
+uschar *clearpass, *s;

 
 /* send a 334, MS Exchange style, and grab the client's request,
 unless we already have it via an initial response. */
 
 /* send a 334, MS Exchange style, and grab the client's request,
 unless we already have it via an initial response. */


@@ -190,6 +190,13 @@ that causes failure if the size of msgbuf is exceeded. ****/
   char * p = (CS responseptr) + IVAL(&responseptr->uUser.offset,0);
   int len = SVAL(&responseptr->uUser.len,0)/2;
 
   char * p = (CS responseptr) + IVAL(&responseptr->uUser.offset,0);
   int len = SVAL(&responseptr->uUser.len,0)/2;
 

+  if (p + len*2 >= CS (responseptr+1))
+    {
+    DEBUG(D_auth)
+      debug_printf("auth_spa_server(): bad uUser spec in response\n");
+    return FAIL;
+    }
+

   if (len + 1 >= sizeof(msgbuf)) return FAIL;
   for (i = 0; i < len; ++i)
     {
   if (len + 1 >= sizeof(msgbuf)) return FAIL;
   for (i = 0; i < len; ++i)
     {


@@ -235,8 +242,15 @@ spa_smb_nt_encrypt(clearpass, challenge.challengeData, ntRespData);
 
 /* compare NT hash (LM may not be available) */
 
 
 /* compare NT hash (LM may not be available) */
 

-if (memcmp(ntRespData, (US responseptr)+IVAL(&responseptr->ntResponse.offset,0),
-      24) == 0)
+s = (US responseptr) + IVAL(&responseptr->ntResponse.offset,0);
+if (s + 24 >= US (responseptr+1))
+  {
+  DEBUG(D_auth)
+    debug_printf("auth_spa_server(): bad ntRespData spec in response\n");
+  return FAIL;
+  }
+
+if (memcmp(ntRespData, s, 24) == 0)

   return auth_check_serv_cond(ablock); /* success. we have a winner. */
 
   /* Expand server_condition as an authorization check (PH) */
   return auth_check_serv_cond(ablock); /* success. we have a winner. */
 
   /* Expand server_condition as an authorization check (PH) */