Commit | Line | Data |
---|---|---|
0c0c20aa AM |
1 | From 031ae594f6e68511117f6d39ce238b0c5215d8d1 Mon Sep 17 00:00:00 2001 |
2 | From: Qualys Security Advisory <qsa@qualys.com> | |
3 | Date: Sun, 21 Feb 2021 22:19:42 -0800 | |
4 | Subject: [PATCH 19/29] Security: Avoid decrement of dkim_collect_input if | |
5 | already at 0 | |
6 | ||
7 | Based on Heiko Schlittermann's commit bf2d6e58. This fixes: | |
8 | ||
9 | 5/ receive_msg() calls dkim_exim_verify_finish(), which sets | |
10 | dkim_collect_input to 0 and calls pdkim_feed_finish(), which calls | |
11 | pdkim_header_complete(), which decreases dkim_collect_input to UINT_MAX, | |
12 | which reactivates the DKIM code. | |
13 | ||
14 | As a result, pdkim_feed() is called again (through receive_getc at the | |
15 | end of receive_msg()), but functions like pdkim_finish_bodyhash() and | |
16 | exim_sha_finish() have already been called (in pdkim_feed_finish()). | |
17 | This suggests a use-after-free. | |
18 | ||
19 | But it seems that a use-after-free would happen only with | |
20 | EVP_DigestFinal() (in exim_sha_finish()), which does not seem to be | |
21 | reachable via DKIM (no SHA3). But we checked OpenSSL only, not GnuTLS. | |
22 | ||
23 | Here is a proof of concept that triggers the bug (which came very close | |
24 | to a security vulnerability): | |
25 | ||
26 | (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\n | |
27 | DKIM-Signature:\r\nXXX'; sleep 30) | nc -n -v 192.168.56.102 25 | |
28 | ||
29 | (gdb) print &dkim_collect_input | |
30 | $2 = (unsigned int *) 0x55e180386d90 <dkim_collect_input> | |
31 | (gdb) watch *(unsigned int *) 0x55e180386d90 | |
32 | ||
33 | Hardware watchpoint 1: *(unsigned int *) 0x55e180386d90 | |
34 | Old value = 0 | |
35 | New value = 4294967295 | |
36 | #0 0x000055e18031f805 in pdkim_header_complete (ctx=ctx@entry=0x55e181b9e8e0) at pdkim.c:1006 | |
37 | #1 0x000055e18032106c in pdkim_feed_finish (ctx=0x55e181b9e8e0, return_signatures=0x55e180386d78 <dkim_signatures>, err=err@entry=0x7ffe443e1d00) at pdkim.c:1490 | |
38 | #2 0x000055e1802a3280 in dkim_exim_verify_finish () at dkim.c:328 | |
39 | #3 0x000055e1802c9d1d in receive_msg (extract_recip=extract_recip@entry=0) at receive.c:3409 | |
40 | --- | |
41 | src/pdkim/pdkim.c | 2 +- | |
42 | 1 file changed, 1 insertion(+), 1 deletion(-) | |
43 | ||
44 | diff --git a/src/pdkim/pdkim.c b/src/pdkim/pdkim.c | |
45 | index e203311da..e3233e9f0 100644 | |
46 | --- a/src/pdkim/pdkim.c | |
47 | +++ b/src/pdkim/pdkim.c | |
48 | @@ -1010,7 +1010,7 @@ else | |
49 | last_sig->next = sig; | |
50 | } | |
51 | ||
52 | - if (--dkim_collect_input == 0) | |
53 | + if (dkim_collect_input && --dkim_collect_input == 0) | |
54 | { | |
55 | ctx->headers = pdkim_prepend_stringlist(ctx->headers, ctx->cur_header->s); | |
56 | ctx->cur_header->s[ctx->cur_header->ptr = 0] = '\0'; | |
57 | -- | |
58 | 2.30.2 | |
59 |