From 85b87bc2af652a81dbb7f12fe0a030f0abdeac4c Mon Sep 17 00:00:00 2001 From: Philip Hazel Date: Wed, 29 Dec 2004 10:55:58 +0000 Subject: [PATCH] Fix buffer overflow vulnerability in spa_base64_to_bits() function. --- doc/doc-txt/ChangeLog | 19 +++++++++++++++---- src/src/auths/auth-spa.c | 13 +++++++++++-- src/src/auths/auth-spa.h | 7 +++++-- src/src/auths/spa.c | 8 ++++---- 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); -- 2.25.1