summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeremy Harris <jgh146exb@wizmail.org>2017-01-28 12:30:29 +0000
committerJeremy Harris <jgh146exb@wizmail.org>2017-01-28 14:24:17 +0000
commit02c4f8fb8927c97939d3daa345148739e275dc8d (patch)
treefed8bb398c66b9b5419423de9416f08524e6a4ec
parent2a27d0e4e61c2eecbb9557e8ad5d0a55077ee4ff (diff)
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
-rw-r--r--src/src/pdkim/pdkim.c57
-rw-r--r--test/log/45065
-rw-r--r--test/scripts/4500-DKIM/45062
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("<NULL>");
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; p<len; p++)
ctx->flags |= 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; p<len; p++)
ctx->cur_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; p<len; p++)
}
else if (ctx->flags & 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)