Bug 867790 (CVE-2012-4527)

Summary: CVE-2012-4527 mcrypt: stack-based buffer overflow by encryption / decryption of overly long file names
Product: [Other] Security Response Reporter: Attila Bogar <attila.bogar>
Component: vulnerabilityAssignee: Red Hat Product Security <security-response-team>
Status: CLOSED ERRATA QA Contact:
Severity: low Docs Contact:
Priority: low    
Version: unspecifiedCC: attila.bogar, contact_redhat, iwamatsu, jlieskov, jrusnack, tcallawa
Target Milestone: ---Keywords: Security
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard: impact=low,public=20121018,reported=20121018,source=researcher,cvss2=4.3/AV:N/AC:M/Au:N/C:N/I:N/A:P,fedora-all/mcrypt=affected,epel-all/mcrypt=affected,cwe=CWE-121[auto]
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-05-09 17:01:37 EDT Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 867879, 867881    
Bug Blocks:    
Attachments:
Description Flags
patch to fix the buffer overflow
none
new spec
none
80-width patch
none
PATH_MAX+1k buffer. Do use buf length in snprintf calls none

Description Attila Bogar 2012-10-18 06:00:02 EDT
Created attachment 629285 [details]
patch to fix the buffer overflow

Description of problem:
A buffer overflow in mcrypt version 2.6.8 and earlier due to long filenames.  If a user were tricked into attempting to encrypt/decrypt specially crafted long filename(s), this flaw would cause a stack-based buffer overflow that could potentially lead to arbitrary code execution.

Note that this is caught by FORTIFY_SOURCE, which renders this to being a crash-only bug on Fedora.

There are currently no upstream patches for this flaw.

Version-Release number of selected component (if applicable):
mcrypt-2.6.8-9.el6 (possibly others too).

How reproducible:
Run mcrypt with ~128 byte long file names.
Comment 1 Attila Bogar 2012-10-18 06:01:26 EDT
Created attachment 629286 [details]
new spec

spec file to build fixed rpms
Comment 2 Jan Lieskovsky 2012-10-18 09:24:43 EDT
Thank you for your report, Attila. I can confirm the issue (looks to be yet something different than CVE-2012-4409):
  https://bugzilla.redhat.com/show_bug.cgi?id=855029

I am going to steal this bug to be security response product one.
Comment 3 Jan Lieskovsky 2012-10-18 09:37:15 EDT
This issue affects the versions of the mcrypt package, as shipped with Fedora release of 16 and 17. Please schedule an update.

--

This issue affects the versions of the mcrypt package, as shipped with Fedora EPEL 5 and Fedora EPEL 6. Please schedule an update.
Comment 4 Jan Lieskovsky 2012-10-18 09:38:21 EDT
Created mcrypt tracking bugs for this issue

Affects: fedora-all [bug 867879]
Affects: epel-all [bug 867881]
Comment 5 Jan Lieskovsky 2012-10-18 09:53:58 EDT
CVE Request:
  http://www.openwall.com/lists/oss-security/2012/10/18/9
Comment 6 Attila Bogar 2012-10-18 10:06:20 EDT
My fix is actually a workaround.

For a proper fix the buffer (tmperr) length should be calculated and malloced before the snprintf is actually called.
Comment 7 Vincent Danen 2012-10-18 16:45:22 EDT
This received CVE-2012-4527:

http://www.openwall.com/lists/oss-security/2012/10/18/12
Comment 8 Tom "spot" Callaway 2012-10-19 14:40:33 EDT
Is there any reason not to apply the workaround patch? I believe that mcrypt is unmaintained at this point in time.
Comment 9 Attila Bogar 2012-10-19 14:55:00 EDT
(In reply to comment #8)
> Is there any reason not to apply the workaround patch? I believe that mcrypt
> is unmaintained at this point in time.

Yes there is.  Someone with more expertise in fixing buffer overflows should come up with a proper patch.

My patch works for me with WIDTH defined 80.  I accidentally submitted the wrong version with WIDTH 132.
Comment 10 Tom "spot" Callaway 2012-10-22 09:18:20 EDT
Well, then I'll just wait for that proper patch.
Comment 11 Nobuhiro Iwamatsu 2012-10-29 18:23:29 EDT
(In reply to comment #9)
> (In reply to comment #8)
> > Is there any reason not to apply the workaround patch? I believe that mcrypt
> > is unmaintained at this point in time.
> 
> Yes there is.  Someone with more expertise in fixing buffer overflows should
> come up with a proper patch.
> 
> My patch works for me with WIDTH defined 80.  I accidentally submitted the
> wrong version with WIDTH 132.

I created the patch which changed WITH into 80. 
Is it OK?
Comment 12 Nobuhiro Iwamatsu 2012-10-29 18:23:58 EDT
Created attachment 635243 [details]
80-width patch
Comment 13 contact_redhat 2012-11-02 14:14:14 EDT
I think it would be better either to replace
#define WIDTH 80
by
#define WIDTH (sizeof(tmperr))

or

#define WIDTH 80
char tmperr[128];
by
#define WIDTH 128
char tmperr[WIDTH];

snprintf does add a \0 at tmperr[WIDTH-1] on overflow

Attached is a proposal to increase the error buffer to PATH_MAX + 1k
The buffer is static, used only in printf. Memory impact should be acceptable.
And this allow to show the end of the error message if a file were to contain a very long name.


I believe a proper fix should be changing all the err_crit like functions to use va_args.
But I suppose it is beyond scope.
Comment 14 contact_redhat 2012-11-02 14:16:43 EDT
Created attachment 637190 [details]
PATH_MAX+1k buffer. Do use buf length in snprintf calls
Comment 15 Tom "spot" Callaway 2012-11-02 14:45:25 EDT
I honestly don't care. I think you need to be proposing this to upstream mcrypt at this point, since the 80 char workaround is sufficient to prevent the overflow.
Comment 16 contact_redhat 2012-11-03 20:25:43 EDT
Just my two cents:

Forget about increasing the buffer size, if you like, ok.

But is weird to have a 128 bytes buffer and use only the 80 firsts bytes.
WIDTH should be 128 to match tmperr buffer.

#define WIDTH 128
char tmperr[WIDTH];

is a better patch, in the sense that it keeps a coherent length where to write compared to the buffer. It make the code reading easier.
Comment 17 Fedora Update System 2012-11-14 13:27:15 EST
mcrypt-2.6.8-10.el6 has been pushed to the Fedora EPEL 6 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 18 Fedora Update System 2012-11-14 13:29:07 EST
mcrypt-2.6.8-10.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.