From ed774df4902eaa5d67f7220a3b2d0831aee2da0f Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Mon, 7 Oct 2024 10:58:14 +0100 Subject: [PATCH] DKIM: fix crash in signing. Bug 3116 Broken-by: 87cb4a166c47 --- doc/doc-txt/ChangeLog | 4 ++++ src/src/miscmods/dkim.c | 32 ++++++++++++++++++++++---------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 68632f516..bd4fd1921 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -63,6 +63,10 @@ JH/13 Bug 3120: Fix parsing of DKIM pubkey DNS record. Previously a crafted record could crash the meesage recieve process. Investigation by Maxim Galaganov. +JH/14 Bug 3116: Fix crash in dkim signing. On kernels supporting immutable + memory segments, a write was done into one when a constant string was + configured for a transport's dkim private key. + Exim version 4.98 ----------------- diff --git a/src/src/miscmods/dkim.c b/src/src/miscmods/dkim.c index 5b18dbcb0..08059f8bf 100644 --- a/src/src/miscmods/dkim.c +++ b/src/src/miscmods/dkim.c @@ -957,10 +957,8 @@ if (dkim_domain) uschar * dkim_canon_expanded; int pdkim_canon; uschar * dkim_sign_headers_expanded = NULL; - uschar * dkim_private_key_expanded; - uschar * dkim_hash_expanded; - uschar * dkim_identity_expanded = NULL; - uschar * dkim_timestamps_expanded = NULL; + uschar * dkim_private_key_expanded, * dkim_hash_expanded; + uschar * dkim_identity_expanded = NULL, * dkim_timestamps_expanded = NULL; unsigned long tval = 0, xval = 0; /* Get canonicalization to use */ @@ -1004,23 +1002,23 @@ if (dkim_domain) if ( dkim_private_key_expanded[0] == '/' && !(dkim_private_key_expanded = expand_file_big_buffer(dkim_private_key_expanded))) - goto bad; + goto clear_key_bad; GET_OPTION("dkim_hash"); if (!(dkim_hash_expanded = expand_string(dkim->dkim_hash))) - { errwhen = US"dkim_hash"; goto expand_bad; } + { errwhen = US"dkim_hash"; goto clear_key_expand_bad; } GET_OPTION("dkim_identity"); if (dkim->dkim_identity) if (!(dkim_identity_expanded = expand_string(dkim->dkim_identity))) - { errwhen = US"dkim_identity"; goto expand_bad; } + { errwhen = US"dkim_identity"; goto clear_key_expand_bad; } else if (!*dkim_identity_expanded) dkim_identity_expanded = NULL; GET_OPTION("dkim_timestamps"); if (dkim->dkim_timestamps) if (!(dkim_timestamps_expanded = expand_string(dkim->dkim_timestamps))) - { errwhen = US"dkim_timestamps"; goto expand_bad; } + { errwhen = US"dkim_timestamps"; goto clear_key_expand_bad; } else { tval = (unsigned long) time(NULL); @@ -1035,8 +1033,11 @@ if (dkim_domain) dkim_hash_expanded, errstr ))) - goto bad; - dkim_private_key_expanded[0] = '\0'; + goto clear_key_bad; + + if (dkim_private_key_expanded != dkim->dkim_private_key) + /* Avoid leaking keying material via big_buffer */ + dkim_private_key_expanded[0] = '\0'; pdkim_set_optional(sig, CS dkim_sign_headers_expanded, @@ -1058,6 +1059,17 @@ if (dkim_domain) while (n->next) n = n->next; n->next = sig; } + continue; /* next selector */ + + clear_key_bad: + if (dkim_private_key_expanded != dkim->dkim_private_key) + dkim_private_key_expanded[0] = '\0'; + goto bad; + + clear_key_expand_bad: + if (dkim_private_key_expanded != dkim->dkim_private_key) + dkim_private_key_expanded[0] = '\0'; + goto expand_bad; } } -- 2.30.2