From 57aa14b216432be381b6295c312065b2fd034f86 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Tue, 5 May 2020 21:02:14 +0100 Subject: [PATCH] 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 1d685a130..6109a14dd 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 fc363df6f..44c99e9f6 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 e7a588dd2..f83d1144a 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) */ -- 2.30.2