oss-security - Re: CVE request - mcrypt buffer overflow flaw

Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 6 Sep 2012 15:11:27 -0500
From: Raphael Geissert <geissert@...ian.org>
To: oss-security@...ts.openwall.com
Subject: Re: CVE request - mcrypt buffer overflow flaw

Hi,

On Thursday 06 September 2012 09:37:14 Vincent Danen wrote:
> A buffer overflow was reported [1],[2] in mcrypt version 2.6.8 and
> earlier due to a boundary error in the processing of an encrypted file
> (via the check_file_head() function in src/extra.c).  If a user were
> tricked into attempting to decrypt a specially-crafted .nc encrypted
> flie, this flaw would cause a stack-based buffer overflow that could
> potentially lead to arbitrary code execution.

I'm attaching a patch that makes mcrypt abort when the salt is longer than 
the temp buffer it uses.

While working on it, I noticed the err_ functions do not have a constant 
printf format, yet there are calls such as:
      sprintf(tmperr, _("Input File: %s\n"), infile);
      err_info(tmperr);
[print_enc_info in src/extra.c]

And a few others in src/mcrypt.c; for instance:
$ mcrypt --no-openpgp "%s.nc" 
mcrypt: h���Fn�`.nc is not a regular file. Skipping...

I'm attaching another patch that prevents the format string attacks.

Cheers,
-- 
Raphael Geissert - Debian Developer
www.debian.org - get.debian.net

diff -urpN mcrypt-2.6.8-1.orig/src/errors.c mcrypt-2.6.8-1/src/errors.c
--- mcrypt-2.6.8-1.orig/src/errors.c	2007-11-07 11:10:19.000000000 -0600
+++ mcrypt-2.6.8-1/src/errors.c	2012-09-06 14:51:57.614219938 -0500
@@ -24,24 +24,24 @@ extern int quiet;
 
 void err_quit(char *errmsg)
 {
-	fprintf(stderr, errmsg);
+	fprintf(stderr, "%s", errmsg);
 	exit(-1);
 }
 
 void err_warn(char *errmsg)
 {
 	if (quiet <= 1)
-		fprintf(stderr, errmsg);
+		fprintf(stderr, "%s", errmsg);
 }
 
 void err_info(char *errmsg)
 {
 	if (quiet == 0)
-		fprintf(stderr, errmsg);
+		fprintf(stderr, "%s", errmsg);
 }
 
 void err_crit(char *errmsg)
 {
 	if (quiet <= 2)
-		fprintf(stderr, errmsg);
+		fprintf(stderr, "%s", errmsg);
 }

diff -urpN mcrypt-2.6.8-1.orig/src/extra.c mcrypt-2.6.8-1/src/extra.c
--- mcrypt-2.6.8-1.orig/src/extra.c	2007-11-07 11:10:20.000000000 -0600
+++ mcrypt-2.6.8-1/src/extra.c	2012-09-06 14:45:34.337011563 -0500
@@ -242,6 +242,8 @@ int check_file_head(FILE * fstream, char
 			if (m_getbit(0, sflag) != 0) { /* if the first bit is set */
 				*salt_size = m_setbit(0, sflag, 0);
 				if (*salt_size > 0) {
+					if (*salt_size > sizeof(tmp_buf))
+					    err_quit(_("Salt is too long\n"));
 					fread(tmp_buf, 1, *salt_size,
 					      fstream);
 					memmove(salt, tmp_buf, *salt_size);

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.