From 20586bdc24045da63928cbdaace2f036e3c80bd5 Mon Sep 17 00:00:00 2001 From: "Heiko Schlittermann (HS12-RIPE)" Date: Wed, 25 Nov 2020 23:19:57 +0100 Subject: [PATCH] SECURITY: Avoid memory corruption in dkim handling Credits: Qualys 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 (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} (cherry picked from commit ea850e27714ccda2090d781ebe89b410bc38c2c6) (cherry picked from commit 4e784efa28b25683c81a857b464777df593cabee) --- src/src/pdkim/pdkim.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c index b4301b5f7..82dab3db1 100644 --- a/src/src/pdkim/pdkim.c +++ b/src/src/pdkim/pdkim.c @@ -722,7 +722,9 @@ If we have to relax the data for this sig, return our copy of it. */ static blob * pdkim_update_ctx_bodyhash(pdkim_bodyhash * b, blob * orig_data, blob * relaxed_data) { -blob * canon_data = orig_data; +const blob const * canon_data = orig_data; +size_t len; + /* Defaults to simple canon (no further treatment necessary) */ if (b->canon_method == PDKIM_CANON_RELAXED) @@ -770,13 +772,13 @@ if (b->canon_method == PDKIM_CANON_RELAXED) if ( b->bodylength >= 0 && b->signed_body_bytes + (unsigned long)canon_data->len > b->bodylength ) - canon_data->len = b->bodylength - b->signed_body_bytes; + len = b->bodylength - b->signed_body_bytes; -if (canon_data->len > 0) +if (len > 0) { - exim_sha_update(&b->body_hash_ctx, CUS canon_data->data, canon_data->len); - b->signed_body_bytes += canon_data->len; - DEBUG(D_acl) pdkim_quoteprint(canon_data->data, canon_data->len); + exim_sha_update(&b->body_hash_ctx, CUS canon_data->data, len); + b->signed_body_bytes += len; + DEBUG(D_acl) pdkim_quoteprint(canon_data->data, len); } return relaxed_data; -- 2.30.2