Gitweb:
https://git.exim.org/exim.git/commitdiff/57aa14b216432be381b6295c312065b2fd034f86
Commit: 57aa14b216432be381b6295c312065b2fd034f86
Parent: 99dbdcf461adff82b2fe68c9c9c690c4982969b1
Author: Jeremy Harris <jgh146exb@???>
AuthorDate: Tue May 5 21:02:14 2020 +0100
Committer: Jeremy Harris <jgh146exb@???>
CommitDate: Tue May 5 21:02:14 2020 +0100
Fix SPA authenticator, checking client-supplied data before using it. Bug 2571
---
doc/doc-txt/ChangeLog | 5 ++
src/src/auths/auth-spa.c | 120 +++++++++++++++++++++++------------------------
src/src/auths/spa.c | 20 ++++++--
3 files changed, 82 insertions(+), 63 deletions(-)
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 1d685a1..6109a14 100644
--- 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.
+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
-----------------
diff --git a/src/src/auths/auth-spa.c b/src/src/auths/auth-spa.c
index fc363df..44c99e9 100644
--- 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) */
{
- 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 */
{
- 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 */
- 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;
- 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 e7a588d..f83d114 100644
--- 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];
-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. */
@@ -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;
+ 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)
{
@@ -235,8 +242,15 @@ spa_smb_nt_encrypt(clearpass, challenge.challengeData, ntRespData);
/* 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) */
