[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Backporting mutt patches to Debian Buster



Attached is a patch that applies to the unpackaged sources of Debian Buster's
version of mutt 1.10.

It includes 3 patches:

	upstream/Fix-rfc2047-base64-decoding-to-abort-on-illegal-char.patch
	debian-specific/Check-for-NULL-userhdrs.patch
	debian-specific/Fix-write_one_header-illegal-header-check.patch

First one applied from Bullseye.  The other two I modified slightly
to match the older sources.

The CVE's make mention of "specially crafted input" but I don't have
any samples to test with.  I only tested that this built on Buster and
opens mail as before.

I have not adjusted any other files but the 3 above and debian/patches/series.
Hopefully this gives a head start on making these fixes available on buster.

- Chris

diff --git a/debian/patches/debian-specific/Check-for-NULL-userhdrs.patch b/debian/patches/debian-specific/Check-for-NULL-userhdrs.patch
new file mode 100644
index 0000000..33f5cb5
--- /dev/null
+++ b/debian/patches/debian-specific/Check-for-NULL-userhdrs.patch
@@ -0,0 +1,56 @@
+From: Chris Frey <cdfrey@foursquare.net>
+Date: Fri, 15 Sep 2023 08:41:00 -0400
+Subject: Check for NULL userhdrs.
+Bug-Debian: https://bugs.debian.org/1051563
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2023-4875
+
+The original patch from Kevin McCarthy backported to Debian buster's
+mutt version 1.10.1-2.1+deb10u6.
+
+Original summary below:
+
+ From: Kevin McCarthy <kevin@8t8.us>
+ Date: Mon, 4 Sep 2023 12:50:07 +0800
+ Subject: Check for NULL userhdrs.
+ Origin: https://gitlab.com/muttmua/mutt/-/commit/4cc3128abdf52c615911589394a03271fddeefc6
+
+ When composing an email, miscellaneous extra headers are stored in a
+ userhdrs list.  Mutt first checks to ensure each header contains at
+ least a colon character, passes the entire userhdr field (name, colon,
+ and body) to the rfc2047 decoder, and safe_strdup()'s the result on
+ the userhdrs list.  An empty result would from the decode would result
+ in a NULL headers being added to list.
+
+ The previous commit removed the possibility of the decoded header
+ field being empty, but it's prudent to add a check to the strchr
+ calls, in case there is another unexpected bug resulting in one.
+
+ Thanks to Chenyuan Mi (@morningbread) for discovering the two strchr
+ crashes, giving a working example draft message, and providing the
+ stack traces for the two NULL derefences.
+ ---
+  sendlib.c | 4 ++--
+  1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/sendlib.c b/sendlib.c
+index f9bf3f9..d75e66a 100644
+--- a/sendlib.c
++++ b/sendlib.c
+@@ -2070,7 +2070,7 @@ int mutt_write_rfc822_header (FILE *fp, ENVELOPE *env, BODY *attach,
+   /* Add any user defined headers */
+   for (; tmp; tmp = tmp->next)
+   {
+-    if ((p = strchr (tmp->data, ':')))
++    if ((p = strchr (NONULL (tmp->data), ':')))
+     {
+       q = p;
+ 
+@@ -2116,7 +2116,7 @@ static void encode_headers (LIST *h)
+ 
+   for (; h; h = h->next)
+   {
+-    if (!(p = strchr (h->data, ':')))
++    if (!(p = strchr (NONULL (h->data), ':')))
+       continue;
+ 
+     i = p - h->data;
diff --git a/debian/patches/debian-specific/Fix-write_one_header-illegal-header-check.patch b/debian/patches/debian-specific/Fix-write_one_header-illegal-header-check.patch
new file mode 100644
index 0000000..8806525
--- /dev/null
+++ b/debian/patches/debian-specific/Fix-write_one_header-illegal-header-check.patch
@@ -0,0 +1,46 @@
+From: Chris Frey <cdfrey@foursquare.net>
+Date: Fri, 15 Sep 2023 10:09:00 -0400
+Subject: Fix write_one_header() illegal header check.
+Bug-Debian: https://bugs.debian.org/1051563
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2023-4874
+
+Backporting original patch from Kevin McCarthy to Debian Buster's
+mutt version 1.10.1-2.1+deb10u6.
+
+Original commentary included below:
+
+ From: Kevin McCarthy <kevin@8t8.us>
+ Date: Sun, 3 Sep 2023 14:11:48 +0800
+ Subject: Fix write_one_header() illegal header check.
+ Origin: https://gitlab.com/muttmua/mutt/-/commit/a4752eb0ae0a521eec02e59e51ae5daedf74fda0
+
+ This is another crash caused by the rfc2047 decoding bug fixed in the
+ second prior commit.
+
+ In this case, an empty header line followed by a header line starting
+ with ":", would result in t==end.
+
+ The mutt_substrdup() further below would go very badly at that point,
+ with t >= end+1.  This could result in either a memcpy onto NULL or a
+ huge malloc call.
+
+ Thanks to Chenyuan Mi (@morningbread) for giving a working example
+ draft message of the rfc2047 decoding flaw.  This allowed me, with
+ further testing, to discover this additional crash bug.
+ ---
+  sendlib.c | 2 +-
+  1 file changed, 1 insertion(+), 1 deletion(-)
+ 
+diff --git a/sendlib.c b/sendlib.c
+index f9bf3f9..d821061 100644
+--- a/sendlib.c
++++ b/sendlib.c
+@@ -1832,7 +1832,7 @@ static int write_one_header (FILE *fp, int pfxw, int max, int wraplen,
+   else
+   {
+     t = strchr (start, ':');
+-    if (!t || t > end)
++    if (!t || t >= end)
+     {
+       dprint (1, (debugfile, "mwoh: warning: header not in "
+ 		  "'key: value' format!\n"));
diff --git a/debian/patches/series b/debian/patches/series
index 0f56a8c..138586a 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -20,3 +20,6 @@ security/CVE-2020-28896.patch
 security/CVE-2021-3181.patch
 upstream/imap-preauth-and-ssh-tunnel.patch
 upstream/Fix-uudecode-buffer-overflow.patch
+upstream/Fix-rfc2047-base64-decoding-to-abort-on-illegal-char.patch
+debian-specific/Check-for-NULL-userhdrs.patch
+debian-specific/Fix-write_one_header-illegal-header-check.patch
diff --git a/debian/patches/upstream/Fix-rfc2047-base64-decoding-to-abort-on-illegal-char.patch b/debian/patches/upstream/Fix-rfc2047-base64-decoding-to-abort-on-illegal-char.patch
new file mode 100644
index 0000000..beca3f7
--- /dev/null
+++ b/debian/patches/upstream/Fix-rfc2047-base64-decoding-to-abort-on-illegal-char.patch
@@ -0,0 +1,45 @@
+From: Kevin McCarthy <kevin@8t8.us>
+Date: Sun, 3 Sep 2023 12:22:01 +0800
+Subject: Fix rfc2047 base64 decoding to abort on illegal characters.
+Origin: https://gitlab.com/muttmua/mutt/-/commit/452ee330e094bfc7c9a68555e5152b1826534555
+Bug-Debian: https://bugs.debian.org/1051563
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2023-4875
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2023-4874
+
+For some reason, the rfc2047 base64 decoder ignored illegal
+characters, instead of aborting.  This seems innocuous, but in fact
+leads to at least three crash-bugs elsewhere in Mutt.
+
+These stem from Mutt, in some cases, passing an entire header
+field (name, colon, and body) to the rfc2047 decoder.  (It is
+technically incorrect to do so, by the way, but is beyond scope for
+these fixes in stable).  Mutt then assumes the result can't be empty
+because of a previous check that the header contains at least a colon.
+
+This commit takes care of the source of the crashes, by aborting the
+rfc2047 decode.  The following two commits add protective fixes to the
+specific crash points.
+
+Thanks to Chenyuan Mi (@morningbread) for discovering the strchr
+crashes, giving a working example draft message, and providing the
+stack traces for the two NULL derefences.
+---
+ rfc2047.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/rfc2047.c b/rfc2047.c
+index 1ce82ebbe49a..36cc76dbc402 100644
+--- a/rfc2047.c
++++ b/rfc2047.c
+@@ -724,7 +724,7 @@ static int rfc2047_decode_word (BUFFER *d, const char *s, char **charset)
+ 	    if (*pp == '=')
+ 	      break;
+ 	    if ((*pp & ~127) || (c = base64val(*pp)) == -1)
+-	      continue;
++              goto error_out_0;
+ 	    if (k + 6 >= 8)
+ 	    {
+ 	      k -= 2;
+-- 
+2.40.1
+

Reply to: