From 02c4f8fb8927c97939d3daa345148739e275dc8d Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sat, 28 Jan 2017 12:30:29 +0000 Subject: [PATCH] DKIM: check pointer to calculated body hash before verify comparison. Bug 2029 We can have a missing body hash from a malformed DKIM-Signature: header --- src/src/pdkim/pdkim.c | 57 +++++++++++++++++-------------------- test/log/4506 | 5 +++- test/scripts/4500-DKIM/4506 | 2 +- 3 files changed, 31 insertions(+), 33 deletions(-) diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c index ad5b1096d..5e8984c6f 100644 --- a/src/src/pdkim/pdkim.c +++ b/src/src/pdkim/pdkim.c @@ -192,7 +192,8 @@ static void pdkim_hexprint(const uschar *data, int len) { int i; -for (i = 0 ; i < len; i++) debug_printf("%02x", data[i]); +if (data) for (i = 0 ; i < len; i++) debug_printf("%02x", data[i]); +else debug_printf(""); debug_printf("\n"); } @@ -803,11 +804,11 @@ for (sig = ctx->sig; sig; sig = sig->next) sig->bodylength = -1; } - /* VERIFICATION --------------------------------------------------------- */ else - { - /* Compare bodyhash */ - if (memcmp(bh.data, sig->bodyhash.data, bh.len) == 0) + /* VERIFICATION --------------------------------------------------------- */ + /* Be careful that the header sig included a bodyash */ + + if (sig->bodyhash.data && memcmp(bh.data, sig->bodyhash.data, bh.len) == 0) { DEBUG(D_acl) debug_printf("PDKIM [%s] Body hash verified OK\n", sig->domain); } @@ -962,27 +963,24 @@ else DKIM_SIGNATURE_HEADERNAME, Ustrlen(DKIM_SIGNATURE_HEADERNAME)) == 0) { - pdkim_signature *new_sig; + pdkim_signature * new_sig, * last_sig; + + /* Create and chain new signature block. We could error-check for all + required tags here, but prefer to create the internal sig and expicitly + fail verification of it later. */ - /* Create and chain new signature block */ DEBUG(D_acl) debug_printf( "PDKIM >> Found sig, trying to parse >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>\n"); - if ((new_sig = pdkim_parse_sig_header(ctx, ctx->cur_header))) + new_sig = pdkim_parse_sig_header(ctx, ctx->cur_header); + + if (!(last_sig = ctx->sig)) + ctx->sig = new_sig; + else { - pdkim_signature *last_sig = ctx->sig; - if (!last_sig) - ctx->sig = new_sig; - else - { - while (last_sig->next) last_sig = last_sig->next; - last_sig->next = new_sig; - } + while (last_sig->next) last_sig = last_sig->next; + last_sig->next = new_sig; } - else - DEBUG(D_acl) debug_printf( - "Error while parsing signature header\n" - "PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n"); } /* all headers are stored for signature verification */ @@ -1003,7 +1001,7 @@ return PDKIM_OK; DLLEXPORT int pdkim_feed(pdkim_ctx *ctx, char *data, int len) { -int p; +int p, rc; /* Alternate EOD signal, used in non-dotstuffing mode */ if (!data) @@ -1028,7 +1026,6 @@ else for (p = 0; pflags |= PDKIM_SEEN_CR; else if (c == '\n') { - int rc; ctx->flags &= ~PDKIM_SEEN_CR; if ((rc = pdkim_bodyline_complete(ctx)) != PDKIM_OK) return rc; @@ -1048,14 +1045,14 @@ else for (p = 0; pcur_header = string_catn(ctx->cur_header, &ctx->cur_header_size, &ctx->cur_header_len, CUS "\r", 1); - if (ctx->flags & PDKIM_SEEN_LF) + if (ctx->flags & PDKIM_SEEN_LF) /* Seen last header line */ { - int rc = pdkim_header_complete(ctx); /* Seen last header line */ - if (rc != PDKIM_OK) return rc; + if ((rc = pdkim_header_complete(ctx)) != PDKIM_OK) + return rc; ctx->flags = ctx->flags & ~(PDKIM_SEEN_LF|PDKIM_SEEN_CR) | PDKIM_PAST_HDRS; DEBUG(D_acl) debug_printf( - "PDKIM >> Body data for hash, canonicalized >>>>>>>>>>>>>>>>>>>>>>\n"); + "PDKIM >> Body data for hash, canonicalized >>>>>>>>>>>>>>>>>>>>>>>>>>>>\n"); continue; } else @@ -1063,11 +1060,9 @@ else for (p = 0; pflags & PDKIM_SEEN_LF) { - if (!(c == '\t' || c == ' ')) - { - int rc = pdkim_header_complete(ctx); /* End of header */ - if (rc != PDKIM_OK) return rc; - } + if (!(c == '\t' || c == ' ')) /* End of header */ + if ((rc = pdkim_header_complete(ctx)) != PDKIM_OK) + return rc; ctx->flags &= ~PDKIM_SEEN_LF; } diff --git a/test/log/4506 b/test/log/4506 index d50bbe1f4..20e9c6b51 100644 --- a/test/log/4506 +++ b/test/log/4506 @@ -4,6 +4,9 @@ 1999-03-02 09:44:33 10HmaX-0005vi-00 DKIM: d=test.ex s=sel c=simple/simple a=rsa-sha1 b=0 [invalid - signature tag missing or invalid] 1999-03-02 09:44:33 10HmaX-0005vi-00 signer: test.ex bits: 0 1999-03-02 09:44:33 10HmaX-0005vi-00 <= CALLER@bloggs.com H=(xxx) [127.0.0.1] P=smtp S=sss id=qwerty1234@disco-zombie.net -1999-03-02 09:44:33 10HmaY-0005vi-00 DKIM: d=test.ex s=sel c=simple/simple a=rsa-sha1 b=1024 [verification failed - body hash mismatch (body probably modified in transit)] +1999-03-02 09:44:33 10HmaY-0005vi-00 DKIM: d=test.ex s=sel c=simple/simple a=rsa-sha1 b=1024 [invalid - signature tag missing or invalid] 1999-03-02 09:44:33 10HmaY-0005vi-00 signer: test.ex bits: 1024 1999-03-02 09:44:33 10HmaY-0005vi-00 <= CALLER@bloggs.com H=(xxx) [127.0.0.1] P=smtp S=sss id=qwerty1234@disco-zombie.net +1999-03-02 09:44:33 10HmaZ-0005vi-00 DKIM: d=test.ex s=sel c=simple/simple a=rsa-sha1 b=1024 [verification failed - body hash mismatch (body probably modified in transit)] +1999-03-02 09:44:33 10HmaZ-0005vi-00 signer: test.ex bits: 1024 +1999-03-02 09:44:33 10HmaZ-0005vi-00 <= CALLER@bloggs.com H=(xxx) [127.0.0.1] P=smtp S=sss id=qwerty1234@disco-zombie.net diff --git a/test/scripts/4500-DKIM/4506 b/test/scripts/4500-DKIM/4506 index b2d53e8fc..14bba8346 100644 --- a/test/scripts/4500-DKIM/4506 +++ b/test/scripts/4500-DKIM/4506 @@ -1,6 +1,6 @@ # DKIM verify, errors # -exim -d-all+acl -DSERVER=server -bd -oX PORT_D +exim -DSERVER=server -bd -oX PORT_D **** # # This should fail verify (missing header hash in sig header) -- 2.30.2