[exim-cvs] SECURITY: DMARC uses From header untrusted data

Top Page
Delete this message
Reply to this message
Author: Exim Git Commits Mailing List
Date:  
To: exim-cvs
Subject: [exim-cvs] SECURITY: DMARC uses From header untrusted data
Gitweb: http://git.exim.org/exim.git/commitdiff/5b7a7c051c9ab9ee7c924a611f90ef2be03e0ad0
Commit:     5b7a7c051c9ab9ee7c924a611f90ef2be03e0ad0
Parent:     69aca2feaca1ebbc55c6f1adaee4738dc328ae90
Author:     Todd Lyons <tlyons@???>
AuthorDate: Mon May 26 12:14:16 2014 -0700
Committer:  Todd Lyons <tlyons@???>
CommitDate: Mon May 26 12:14:16 2014 -0700


    SECURITY: DMARC uses From header untrusted data


    CVE-2014-2957


    To find the sending domain, expand_string() was used to directly parse
      the contents of the From header. This passes untrusted data directly
      into an internal function. Convert to use standard internal parsing
      functions.
---
 src/src/dmarc.c |   43 ++++++++++++++++++++++++-------------------
 1 files changed, 24 insertions(+), 19 deletions(-)


diff --git a/src/src/dmarc.c b/src/src/dmarc.c
index 6e51652..c619061 100644
--- a/src/src/dmarc.c
+++ b/src/src/dmarc.c
@@ -168,26 +168,31 @@ int dmarc_process() {
     dmarc_abort = TRUE;
   else
   {
-    /* I strongly encourage anybody who can make this better to contact me directly!
-     * <cannonball> Is this an insane way to extract the email address from the From: header?
-     * <jgh_hm> it's sure a horrid layer-crossing....
-     * <cannonball> I'm not denying that :-/
-     * <jgh_hm> there may well be no better though
-     */
-    header_from_sender = expand_string(
-                           string_sprintf("${domain:${extract{1}{:}{${addresses:%s}}}}",
-                             from_header->text) );
-    /* The opendmarc library extracts the domain from the email address, but
-     * only try to store it if it's not empty.  Otherwise, skip out of DMARC. */
-    if (strcmp( CCS header_from_sender, "") == 0)
-      dmarc_abort = TRUE;
-    libdm_status = (dmarc_abort == TRUE) ?
-               DMARC_PARSE_OKAY :
-           opendmarc_policy_store_from_domain(dmarc_pctx, header_from_sender);
-    if (libdm_status != DMARC_PARSE_OKAY)
+  uschar * errormsg;
+  int dummy, domain;
+  uschar * p;
+  uschar saveend;
+
+  parse_allow_group = TRUE;
+  p = parse_find_address_end(from_header->text, FALSE);
+  saveend = *p; *p = '\0';
+  if ((header_from_sender = parse_extract_address(from_header->text, &errormsg,
+                              &dummy, &dummy, &domain, FALSE)))
+    header_from_sender += domain;
+  *p = saveend;
+
+  /* The opendmarc library extracts the domain from the email address, but
+   * only try to store it if it's not empty.  Otherwise, skip out of DMARC. */
+  if (!header_from_sender || (strcmp( CCS header_from_sender, "") == 0))
+    dmarc_abort = TRUE;
+  libdm_status = dmarc_abort ?
+    DMARC_PARSE_OKAY :
+    opendmarc_policy_store_from_domain(dmarc_pctx, header_from_sender);
+  if (libdm_status != DMARC_PARSE_OKAY)
     {
-      log_write(0, LOG_MAIN|LOG_PANIC, "failure to store header From: in DMARC: %s, header was '%s'",
-                           opendmarc_policy_status_to_str(libdm_status), from_header->text);
+      log_write(0, LOG_MAIN|LOG_PANIC,
+                "failure to store header From: in DMARC: %s, header was '%s'",
+                opendmarc_policy_status_to_str(libdm_status), from_header->text);
       dmarc_abort = TRUE;
     }
   }