[exim-cvs] SECURITY: Avoid modification of constant data in …

Gitweb: https://git.exim.org/exim.git/commitdiff/55ab0c211c3e0def1016971d3b3ebd47c006a751

Commit:     55ab0c211c3e0def1016971d3b3ebd47c006a751
Parent:     60482d5517a46f8ee1629f58b39e6897b9b27a98
Author:     Heiko Schlittermann (HS12-RIPE) <hs@???>
AuthorDate: Tue Mar 30 22:59:25 2021 +0200
Committer:  Heiko Schlittermann (HS12-RIPE) <hs@???>
CommitDate: Thu May 27 21:30:56 2021 +0200

    SECURITY: Avoid modification of constant data in dkim handling

    Based on Heiko Schlittermann's commits f880c7f3 and c118c7f4. This
    fixes:

    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 92359a62a0e31734ad8069c66f64b37f9eaaccbe)
    (cherry picked from commit c5f2f5cf2a6b45ae7ba0ed15e04fbe014727b210)
---
 src/src/pdkim/pdkim.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c
index 074106b..45fa2a1 100644
--- a/src/src/pdkim/pdkim.c
+++ b/src/src/pdkim/pdkim.c
@@ -107,7 +107,7 @@ pdkim_combined_canon_entry pdkim_combined_canons[] = {
};


-static blob lineending = {.data = US"\r\n", .len = 2};
+static const blob lineending = {.data = US"\r\n", .len = 2};

/* -------------------------------------------------------------------------- */
uschar *
@@ -720,9 +720,9 @@ 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, const blob const * orig_data, blob * relaxed_data)
+pdkim_update_ctx_bodyhash(pdkim_bodyhash * b, const blob * orig_data, blob * relaxed_data)
{
-const blob const * canon_data = orig_data;
+const blob * canon_data = orig_data;
size_t left;

 /* Defaults to simple canon (no further treatment necessary) */
@@ -771,9 +771,9 @@ 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 + left > b->bodylength
+   && left > (unsigned long)b->bodylength - b->signed_body_bytes
    )
-  left = b->bodylength - b->signed_body_bytes;
+  left = (unsigned long)b->bodylength - b->signed_body_bytes;

if (left > 0)
{