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.

diff --git a/src/src/dmarc.c b/src/src/dmarc.c
index 6e516525fd6b123625e496f718346a72b566af18..c6190613e928d7a9882636a3a07e320f80f65e96 100644 (file)
--- a/src/src/dmarc.c
+++ b/src/src/dmarc.c
@@ -168,26 +168,31 @@ int dmarc_process() {
     dmarc_abort = TRUE;
   else
   {
     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;
     }
   }
       dmarc_abort = TRUE;
     }
   }