From 5b7a7c051c9ab9ee7c924a611f90ef2be03e0ad0 Mon Sep 17 00:00:00 2001 From: Todd Lyons Date: Mon, 26 May 2014 12:14:16 -0700 Subject: 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 file changed, 24 insertions(+), 19 deletions(-) diff --git a/src/src/dmarc.c b/src/src/dmarc.c index 6e516525f..c6190613e 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! - * Is this an insane way to extract the email address from the From: header? - * it's sure a horrid layer-crossing.... - * I'm not denying that :-/ - * 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; } } -- cgit v1.2.3