[exim-cvs] DKIM: fix crash in signing. Bug 3116

Top Page
Delete this message
Reply to this message
Author: Exim Git Commits Mailing List
Date:  
To: exim-cvs
Subject: [exim-cvs] DKIM: fix crash in signing. Bug 3116
Gitweb: https://git.exim.org/exim.git/commitdiff/ed774df4902eaa5d67f7220a3b2d0831aee2da0f
Commit:     ed774df4902eaa5d67f7220a3b2d0831aee2da0f
Parent:     78e481bba25df9e8e0cc4308783d1dbc5d58dc71
Author:     Jeremy Harris <jgh146exb@???>
AuthorDate: Mon Oct 7 10:58:14 2024 +0100
Committer:  Jeremy Harris <jgh146exb@???>
CommitDate: Fri Nov 1 13:17:17 2024 +0000


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



--
## subscription configuration (requires account):
## https://lists.exim.org/mailman3/postorius/lists/exim-cvs.lists.exim.org/
## unsubscribe (doesn't require an account):
## exim-cvs-unsubscribe@???
## Exim details at http://www.exim.org/
## Please use the Wiki with this list - http://wiki.exim.org/