From: Jeremy Harris <jgh146exb@wizmail.org>
Date: Mon, 7 Oct 2024 09:58:14 +0000 (+0100)
Subject: DKIM: fix crash in signing.  Bug 3116
X-Git-Url: https://git.exim.org/exim.git/commitdiff_plain/ed774df4902eaa5d67f7220a3b2d0831aee2da0f

DKIM: fix crash in signing.  Bug 3116

Broken-by: 87cb4a166c47
---

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;
     }
   }