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.
Created attachment 629286 [details] new spec spec file to build fixed rpms
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.
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.
Created mcrypt tracking bugs for this issue Affects: fedora-all [bug 867879] Affects: epel-all [bug 867881]
CVE Request: http://www.openwall.com/lists/oss-security/2012/10/18/9
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.
This received CVE-2012-4527: http://www.openwall.com/lists/oss-security/2012/10/18/12
Is there any reason not to apply the workaround patch? I believe that mcrypt is unmaintained at this point in time.
(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.
Well, then I'll just wait for that proper patch.
(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?
Created attachment 635243 [details] 80-width patch
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.
Created attachment 637190 [details] PATH_MAX+1k buffer. Do use buf length in snprintf calls
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.
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.
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.
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.