--- /dev/null
+Date: Thu, 29 Oct 2020 18:45:45 +0000
+From: Qualys Security Advisory via Security <security@exim.org>
+To: "security@exim.org" <security@exim.org>
+Subject: Re: [Exim-Security] Audit
+Sender: Security <security-bounces+hs=nodmarc.schlittermann.de@exim.org>
+Return-Path: <security-bounces+hs=nodmarc.schlittermann.de@exim.org>
+Authentication-Results: mx10.schlittermann.de; spf=pass
+ smtp.mailfrom=exim.org; dkim=pass header.d=exim.org header.s=d202008
+ header.a=rsa-sha256; dmarc=none header.from=exim.org
+Authentication-Results: exim.org; iprev=pass (mx0a-001ca501.pphosted.com)
+ smtp.remote-ip=148.163.156.198; spf=pass smtp.mailfrom=qualys.com; dkim=pass
+ header.d=qualys.com header.s=qualyscom header.a=rsa-sha256; dkim=pass
+ header.d=qualys.onmicrosoft.com header.s=selector2-qualys-onmicrosoft-com
+ header.a=rsa-sha256; dmarc=pass header.from=qualys.com; arc=pass (i=1)
+ header.s=arcselector9901 arc.oldest-pass=1 smtp.remote-ip=148.163.156.198
+Authentication-Results: ppops.net; spf=pass smtp.mailfrom=qsa@qualys.com
+authentication-results: exim.org; dkim=none (message not signed)
+ header.d=none;exim.org; dmarc=none action=none header.from=qualys.com;
+X-Spam-Score: 0.0 (/)
+Reply-To: Qualys Security Advisory <qsa@qualys.com>
+
+Hi all,
+
+Below are various non-security bugs that we found during our audit.
+First, a few bugs that almost have security consequences.
+
+========================================================================
+
+1/ In src/transports/smtp.c:
+
+2281 int n = sizeof(sx->buffer);
+2282 uschar * rsp = sx->buffer;
+2283
+2284 if (sx->esmtp_sent && (n = Ustrlen(sx->buffer)) < sizeof(sx->buffer)/2)
+2285 { rsp = sx->buffer + n + 1; n = sizeof(sx->buffer) - n; }
+
+This should probably be either:
+
+rsp = sx->buffer + n + 1; n = sizeof(sx->buffer) - n - 1;
+
+or:
+
+rsp = sx->buffer + n; n = sizeof(sx->buffer) - n;
+
+(not sure which) to avoid an off-by-one.
+
+========================================================================
+
+2/ In src/spool_in.c:
+
+ 462 while ( (len = Ustrlen(big_buffer)) == big_buffer_size-1
+ 463 && big_buffer[len-1] != '\n'
+ 464 )
+ 465 { /* buffer not big enough for line; certs make this possible */
+ 466 uschar * buf;
+ 467 if (big_buffer_size >= BIG_BUFFER_SIZE*4) goto SPOOL_READ_ERROR;
+ 468 buf = store_get_perm(big_buffer_size *= 2, FALSE);
+ 469 memcpy(buf, big_buffer, --len);
+
+The --len in memcpy() chops off a useful byte (we know for sure that
+big_buffer[len-1] is not a '\n' because we entered the while loop).
+
+========================================================================
+
+3/ In src/deliver.c:
+
+ 333 static int
+ 334 open_msglog_file(uschar *filename, int mode, uschar **error)
+ 335 {
+ 336 if (Ustrstr(filename, US"/../"))
+ 337 log_write(0, LOG_MAIN|LOG_PANIC,
+ 338 "Attempt to open msglog file path with upward-traversal: '%s'\n", filename);
+
+Should this be LOG_PANIC_DIE instead of LOG_PANIC? Right now it will log
+the /../ attempt but will open the file anyway.
+
+========================================================================
+
+4/ In src/smtp_in.c:
+
+4966 case RCPT_CMD:
+4967 HAD(SCH_RCPT);
+4968 rcpt_count++;
+....
+5123 if (rcpt_count > recipients_max && recipients_max > 0)
+
+In theory this recipients_max check can be bypassed, because the int
+rcpt_count can overflow (become negative). In practice this would either
+consume too much memory or generate too much network traffic, but maybe
+it should be fixed anyway.
+
+========================================================================
+
+5/ receive_msg() calls dkim_exim_verify_finish(), which sets
+dkim_collect_input to 0 and calls pdkim_feed_finish(), which calls
+pdkim_header_complete(), which decreases dkim_collect_input to UINT_MAX,
+which reactivates the DKIM code.
+
+As a result, pdkim_feed() is called again (through receive_getc at the
+end of receive_msg()), but functions like pdkim_finish_bodyhash() and
+exim_sha_finish() have already been called (in pdkim_feed_finish()).
+This suggests a use-after-free.
+
+But it seems that a use-after-free would happen only with
+EVP_DigestFinal() (in exim_sha_finish()), which does not seem to be
+reachable via DKIM (no SHA3). But we checked OpenSSL only, not GnuTLS.
+
+Here is a proof of concept that triggers the bug (which came very close
+to a security vulnerability):
+
+(sleep 10; echo 'EHLO test'; sleep 3; echo 'MAIL FROM:<>'; sleep 3; echo 'RCPT TO:postmaster'; sleep 3; echo 'BDAT 42 LAST'; date >&2; sleep 30; printf 'not a valid header line\r\nDKIM-Signature:\r\nXXX'; sleep 30) | nc -n -v 192.168.56.102 25
+
+(gdb) print &dkim_collect_input
+$2 = (unsigned int *) 0x55e180386d90 <dkim_collect_input>
+(gdb) watch *(unsigned int *) 0x55e180386d90
+
+Hardware watchpoint 1: *(unsigned int *) 0x55e180386d90
+Old value = 0
+New value = 4294967295
+#0 0x000055e18031f805 in pdkim_header_complete (ctx=ctx@entry=0x55e181b9e8e0) at pdkim.c:1006
+#1 0x000055e18032106c in pdkim_feed_finish (ctx=0x55e181b9e8e0, return_signatures=0x55e180386d78 <dkim_signatures>, err=err@entry=0x7ffe443e1d00) at pdkim.c:1490
+#2 0x000055e1802a3280 in dkim_exim_verify_finish () at dkim.c:328
+#3 0x000055e1802c9d1d in receive_msg (extract_recip=extract_recip@entry=0) at receive.c:3409
+
+========================================================================
+
+6/ In src/pdkim/pdkim.c, pdkim_update_ctx_bodyhash() is sometimes called
+with a global orig_data and hence canon_data, and the following line can
+therefore modify data that should be constant:
+
+ 773 canon_data->len = b->bodylength - b->signed_body_bytes;
+
+For example, the following proof of concept sets lineending.len to 0
+(this should not be possible):
+
+(sleep 10; echo 'EHLO test'; sleep 3; echo 'MAIL FROM:<>'; sleep 3; echo 'RCPT TO:postmaster'; sleep 3; echo 'DATA'; date >&2; sleep 30; printf 'DKIM-Signature:a=rsa-sha1;c=simple/simple;l=0\r\n\r\n\r\nXXX\r\n.\r\n'; sleep 30) | nc -n -v 192.168.56.102 25
+
+(gdb) print lineending
+$1 = {data = 0x55e18035b2ad "\r\n", len = 2}
+(gdb) print &lineending.len
+$3 = (size_t *) 0x55e180385948 <lineending+8>
+(gdb) watch *(size_t *) 0x55e180385948
+
+Hardware watchpoint 1: *(size_t *) 0x55e180385948
+Old value = 2
+New value = 0
+(gdb) print lineending
+$5 = {data = 0x55e18035b2ad "\r\n", len = 0}
+
+========================================================================
+
+7/ In src/smtp_out.c, read_response_line(), inblock->ptr is not updated
+when -1 is returned. This does not seem to have bad consequences, but is
+maybe not the intended behavior.
+
+========================================================================
+
+8/ When gstring_grow() calls store_extend(), oldsize is g->size, and
+store_extend() therefore applies its rounding to g->size. But initially,
+the rounding was not applied to g->size but to sizeof(gstring) + g->size
+(in string_get()). This luckily does not have bad consequences on 64-bit
+(because sizeof(gstring) is rounded -- a multiple of alignment), but on
+32-bit it may have unintended consequences. We were not able to exploit
+this, however.
+
+========================================================================
+
+9/ In several cases a fixed gstring is built manually, but is then used
+with string functions that may grow/extend the gstring and have
+unintended consequences. For example, in src/exim.c:
+
+ 180 void
+ 181 set_process_info(const char *format, ...)
+ 182 {
+ 183 gstring gs = { .size = PROCESS_INFO_SIZE - 2, .ptr = 0, .s = process_info };
+ 184 gstring * g;
+ 185 int len;
+ 186 va_list ap;
+ 187
+ 188 g = string_fmt_append(&gs, "%5d ", (int)getpid());
+ 189 len = g->ptr;
+ 190 va_start(ap, format);
+ 191 if (!string_vformat(g, 0, format, ap))
+ 192 {
+ 193 gs.ptr = len;
+ 194 g = string_cat(&gs, US"**** string overflowed buffer ****");
+ 195 }
+ 196 g = string_catn(g, US"\n", 1);
+
+string_fmt_append() uses the SVFMT_EXTEND flag, and string_cat() and
+string_catn() call gstring_grow(). We were not able to exploit this,
+however.
+
+========================================================================
+
+10/ string_vformat_trc() should always call die_tainted() if the format
+string is tainted. Otherwise an attacker can exploit %n or overflow the
+newformat[] stack buffer.
+
+One note about untainted memory vs. memory corruption: the ideal
+long-term solution would be to make all untainted memory read-only
+(through mprotect()). Not sure if this would be possible or portable,
+but we wanted to at least mention it.
+
+========================================================================
+
+Finally, a few minor bugs/questions.
+
+========================================================================
+
+11/ In src/pdkim/pdkim.c, pdkim_parse_sig_header(), there might be a
+missing break at the end of the case 'a'.
+
+========================================================================
+
+12/ Same function, the call to pdkim_strtrim() seems to be useless
+because whitespace is already skipped by:
+
+ 531 if (c == '\r' || c == '\n' || c == ' ' || c == '\t')
+ 532 goto NEXT_CHAR;
+
+But we have not fully analyzed this, so we might be missing something.
+
+========================================================================
+
+13/ Same file, HEADER_BUFFER_FRAG_SIZE seems to be unused?
+
+========================================================================
+
+14/ In src/smtp_in.c, bdat_getc(), dkim_exim_verify_feed() is called but
+does nothing because dkim_collect_input is always set to 0 at the
+beginning of bdat_getc().
+
+========================================================================
+
+We are at your disposal for questions, comments, and further
+discussions. Thank you very much!
+
+With best regards,
+
+--
+the Qualys Security Advisory team
+
+
+[https://d1dejaj6dcqv24.cloudfront.net/asset/image/email-banner-384-2x.png]<https://www.qualys.com/email-banner>
+
+
+
+This message may contain confidential and privileged information. If it has been sent to you in error, please reply to advise the sender of the error and then immediately delete it. If you are not the intended recipient, do not read, copy, disclose or otherwise use this message. The sender disclaims any liability for such unauthorized use. NOTE that all incoming emails sent to Qualys email accounts will be archived and may be scanned by us and/or by external service providers to detect and prevent threats to our systems, investigate illegal or inappropriate behavior, and/or eliminate unsolicited promotional emails (“spam”). If you have any concerns about this process, please contact us.
+_______________________________________________
+Security mailing list
+Security@exim.org
+https://lists.exim.org/mailman/listinfo/security
git://git.exim.org / exim-website.git / blobdiff