Date: Tue, 2 Oct 2012 13:42:44 -0500 From: Raphael Geissert <geissert@...ian.org> To: Kurt Seifried <kseifried@...hat.com> Cc: oss-security@...ts.openwall.com Subject: Re: CVE request - mcrypt buffer overflow flaw Kurt, I think at least one more CVE id needs to be assigned: On Saturday 15 September 2012 19:22:06 Raphael Geissert wrote: > On Tuesday 11 September 2012 10:19:38 Eygene Ryabinkin wrote: > > Unfortunately, mcrypt's check_file_head() in combination with > > decrypt_general() is a bit worse: it allows to overwrite up to 50 > > bytes of stack buffers from decrypt_general(), namely local_algorithm, > > local_mode, local_keymode. And in some curcumstances to overwrite > > even 2-3 extra bytes (not more, since buf will contain '\0'), though > > it is not very much controllable path. > > > > The problem is that no length checks are done in combos > > read_until_null/strcpy. Function read_until_null() allows for up to > > 100 bytes to be read and it won't NUL-terminate the buffer, so strcpy > > can do perform access even further (read from tmp_buf and writes to > > the said buffers; but this is the uncontrolled way I was talking > > about). > > > > The modified PoC is at > > > > http://codelabs.ru/security/mcrypt/poc-cve-2012-4409.py > > > > With it I was able to overwrite the salt_size@...rypt_general() > > and to trigger the call to malloc() for the chunk of 0x42424242 bytes > > via _mcrypt_malloc() that lead to bus error because of subsequent > > memmove(): > [...] > > > I wasn't yet able to smash the stack of decrypt_general(), because > > BUFFER_SIZE is 1024 and tmp_buf prevents me to reach the top of the > > stack frame (provided that compiler won't rearrange local variables), > > so I was not able to go past it. Thus it looks like a temporary > > memory consumption/DoS. > > Another week, another couple of patches. One makes it use strncpy and > forces a NUL on the last byte of local_algorithm, local_mode, and > local_keymode. Their values are checked later on, so it seems safe to > pass unvalidated data. > The size of the buffers is hard-coded to avoid making many changes to the > code. I think this needs a separate id, since fixes were released by Fedora and Debian referencing CVE-2012-4409 but only for the original report. Eygene's followup issues have been fixed in Debian without referencing a CVE id. > Once those issues were fixed I noticed that salt_size is not initialized > if the salt flag is not set. The result is an inconditional call to > malloc, with an uninitialized int as argument. This can lead to a > non-attacker-controlled memory consumption DoS in most cases. > It makes me think nobody actually ever used it without a salt. I've no strong opinion on whether this deserves an id. Cheers, -- Raphael Geissert - Debian Developer www.debian.org - get.debian.net
Powered by blists - more mailing lists
Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.
Powered by Openwall GNU/*/Linux - Powered by OpenVZ