summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHeiko Schlittermann (HS12-RIPE) <hs@schlittermann.de>2020-11-26 18:16:59 +0100
committerHeiko Schlittermann (HS12-RIPE) <hs@schlittermann.de>2021-04-28 00:40:33 +0200
commit667fb25b8f0dc3fbac57bce4051e345555fa776a (patch)
tree8f940a9258d3fced470236ab1c708cf0dbfb6479
parent4e784efa28b25683c81a857b464777df593cabee (diff)
downloadexim4-667fb25b8f0dc3fbac57bce4051e345555fa776a.tar.gz
SECURITY: Avoid modification of constant data
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 <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} (cherry picked from commit 9fce76f56459dde7489eb21ce1ff822e04e10f43)
-rw-r--r--src/src/pdkim/pdkim.c17
1 files changed, 9 insertions, 8 deletions
diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c
index da2ea7e49..4c73d4fac 100644
--- a/src/src/pdkim/pdkim.c
+++ b/src/src/pdkim/pdkim.c
@@ -720,10 +720,10 @@ return NULL;
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)
+pdkim_update_ctx_bodyhash(pdkim_bodyhash * b, const blob const * orig_data, blob * relaxed_data)
{
const blob const * canon_data = orig_data;
-size_t len;
+size_t left;
/* Defaults to simple canon (no further treatment necessary) */
@@ -769,16 +769,17 @@ if (b->canon_method == PDKIM_CANON_RELAXED)
}
/* Make sure we don't exceed the to-be-signed body length */
+left = canon_data->len;
if ( b->bodylength >= 0
- && b->signed_body_bytes + (unsigned long)canon_data->len > b->bodylength
+ && b->signed_body_bytes + left > b->bodylength
)
- len = b->bodylength - b->signed_body_bytes;
+ left = b->bodylength - b->signed_body_bytes;
-if (len > 0)
+if (left > 0)
{
- 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);
+ exim_sha_update(&b->body_hash_ctx, CUS canon_data->data, left);
+ b->signed_body_bytes += left;
+ DEBUG(D_acl) pdkim_quoteprint(canon_data->data, left);
}
return relaxed_data;