diff options
author | Heiko Schlittermann (HS12-RIPE) <hs@schlittermann.de> | 2020-11-26 18:16:59 +0100 |
---|---|---|
committer | Heiko Schlittermann (HS12-RIPE) <hs@schlittermann.de> | 2021-05-27 21:30:39 +0200 |
commit | 873db3a8000c99e3f0d0a62bb9f70672e1268047 (patch) | |
tree | 31d1acd025203e637f493627b6c197266b75851d /src | |
parent | 20586bdc24045da63928cbdaace2f036e3c80bd5 (diff) |
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)
(cherry picked from commit 667fb25b8f0dc3fbac57bce4051e345555fa776a)
Diffstat (limited to 'src')
-rw-r--r-- | src/src/pdkim/pdkim.c | 17 |
1 files changed, 9 insertions, 8 deletions
diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c index 82dab3db1..0ad4d12d8 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; |