From 6ce5c70cff8989418e05d01fd2a57703007a6357 Mon Sep 17 00:00:00 2001
From: Jeremy Harris <jgh146exb@wizmail.org>
Date: Mon, 1 Jul 2024 19:35:12 +0100
Subject: [PATCH] Fix MIME parsing of filenames specified using multiple
 parameters.  Bug 3099

---
 doc/ChangeLog |  3 +++
 doc/spec.txt  | 10 +++++-----
 src/mime.c    | 51 +++++++++++++++++++++++++++++----------------------
 src/string.c  |  1 +
 4 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/doc/ChangeLog b/doc/ChangeLog
index c88454c..635f408 100644
--- a/doc/ChangeLog
+++ b/doc/ChangeLog
@@ -221,6 +221,9 @@ JH/43 Bug 2903: avoid exit on an attempt to rewrite a malformed address.
 JH/44 Bug 3033: Harden dnsdb lookups against crafted DNS responses.
       CVE-2023-42219
 
+JH/45 Bug 3099: fix parsing of MIME filenames split over multiple paramemters.
+      Previously the $mime_filename variable would have an incorrect value.
+
 HS/02 Fix string_is_ip_address() CVE-2023-42117 (Bug 3031)
 
 Exim version 4.96
diff --git a/doc/spec.txt b/doc/spec.txt
index 6bb656e..8f598e7 100644
--- a/doc/spec.txt
+++ b/doc/spec.txt
@@ -32280,13 +32280,13 @@ The right hand side is expanded before use. After expansion, the value can be:
     the default path is then used.
 
 The decode condition normally succeeds. It is only false for syntax errors or
-unusual circumstances such as memory shortages. You can easily decode a file
-with its original, proposed filename using
+errors or unusual circumstances such as memory shortages.
 
-decode = $mime_filename
+The variable &$mime_filename$& will have the suggested name for the file.
+Note however that this might contain anything, and is very difficult
+to safely use as all or even part of the filename.
 
-However, you should keep in mind that $mime_filename might contain anything. If
-you place files outside of the default path, they are not automatically
+If you place files outside of the default path, they are not
 unlinked.
 
 For RFC822 attachments (these are messages attached to messages, with a
diff --git a/src/mime.c b/src/mime.c
index 975ddca..5f9e1ad 100644
--- a/src/mime.c
+++ b/src/mime.c
@@ -587,10 +587,10 @@ while(1)
 
 	while (*p)
 	  {
-	  DEBUG(D_acl) debug_printf_indent("MIME:   considering paramlist '%s'\n", p);
+	  DEBUG(D_acl)
+	    debug_printf_indent("MIME:   considering paramlist '%s'\n", p);
 
-	  if (  !mime_filename
-	     && strncmpic(CUS"content-disposition:", header, 20) == 0
+	  if (  strncmpic(CUS"content-disposition:", header, 20) == 0
 	     && strncmpic(CUS"filename*", p, 9) == 0
 	     )
 	    {					/* RFC 2231 filename */
@@ -604,11 +604,12 @@ while(1)
 
 	    if (q && *q)
 	      {
-	      uschar * temp_string, * err_msg;
+	      uschar * temp_string, * err_msg, * fname = q;
 	      int slen;
 
 	      /* build up an un-decoded filename over successive
 	      filename*= parameters (for use when 2047 decode fails) */
+/*XXX could grow a gstring here */
 
 	      mime_fname_rfc2231 = string_sprintf("%#s%s",
 		mime_fname_rfc2231, q);
@@ -623,26 +624,32 @@ while(1)
 		  /* look for a ' in the "filename" */
 		  while(*s != '\'' && *s) s++;	/* s is 1st ' or NUL */
 
-		  if ((size = s-q) > 0)
-		    mime_filename_charset = string_copyn(q, size);
+		  if (*s)			/* there was a ' */
+		    {
+		    if ((size = s-q) > 0)
+		      mime_filename_charset = string_copyn(q, size);
 
-		  if (*(p = s)) p++;
-		  while(*p == '\'') p++;	/* p is after 2nd ' */
+		    if (*(fname = s)) fname++;
+		    while(*fname == '\'') fname++;    /* fname is after 2nd ' */
+		    }
 		  }
-		else
-		  p = q;
 
-		DEBUG(D_acl) debug_printf_indent("MIME:    charset %s fname '%s'\n",
-		  mime_filename_charset ? mime_filename_charset : US"<NULL>", p);
+		DEBUG(D_acl)
+		  debug_printf_indent("MIME:    charset %s fname '%s'\n",
+		    mime_filename_charset ? mime_filename_charset : US"<NULL>",
+		    fname);
 
-		temp_string = rfc2231_to_2047(p, mime_filename_charset, &slen);
-		DEBUG(D_acl) debug_printf_indent("MIME:    2047-name %s\n", temp_string);
+		temp_string = rfc2231_to_2047(fname, mime_filename_charset,
+					      &slen);
+		DEBUG(D_acl)
+		  debug_printf_indent("MIME:    2047-name %s\n", temp_string);
 
 		temp_string = rfc2047_decode(temp_string, FALSE, NULL, ' ',
-		  NULL, &err_msg);
-		DEBUG(D_acl) debug_printf_indent("MIME:    plain-name %s\n", temp_string);
+					      NULL, &err_msg);
+		DEBUG(D_acl)
+		  debug_printf_indent("MIME:    plain-name %s\n", temp_string);
 
-		if (!temp_string || (size = Ustrlen(temp_string))  == slen)
+		if (!temp_string || (size = Ustrlen(temp_string)) == slen)
 		  decoding_failed = TRUE;
 		else
 		  /* build up a decoded filename over successive
@@ -651,9 +658,9 @@ while(1)
 		  mime_filename = mime_fname = mime_fname
 		    ? string_sprintf("%s%s", mime_fname, temp_string)
 		    : temp_string;
-		}
-	      }
-	    }
+		}	/*!decoding_failed*/
+	      }		/*q*/
+	    }		/*2231 filename*/
 
 	  else
 	    /* look for interesting parameters */
@@ -682,7 +689,7 @@ while(1)
 
 
 	  /* There is something, but not one of our interesting parameters.
-	     Advance past the next semicolon */
+	  Advance past the next semicolon */
 	  p = mime_next_semicolon(p);
 	  if (*p) p++;
 	  }				/* param scan on line */
@@ -800,5 +807,5 @@ return rc;
 
 #endif	/*WITH_CONTENT_SCAN*/
 
-/* vi: sw ai sw=2
+/* vi: aw ai sw=2
 */
diff --git a/src/string.c b/src/string.c
index dfe0f24..2f77cc7 100644
--- a/src/string.c
+++ b/src/string.c
@@ -1347,6 +1347,7 @@ Field width:		decimal digits, or *
 Precision:		dot, followed by decimal digits or *
 Length modifiers:	h  L  l  ll  z
 Conversion specifiers:	n d o u x X p f e E g G % c s S T Y D M
+Alternate-form:                %#s is silent about a null string
 
 Returns the possibly-new (if copy for growth or taint-handling was needed)
 string, not nul-terminated.
-- 
2.33.0