Re: CVE request - mcrypt buffer overflow flaw

[Raphael Geissert <geissert () debian org>]

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[3] 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@decrypt_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.

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.

Once again, I've not taken a look at other bits of mcrypt, so there might be 
more issues around.

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

Attachment: uninitialized_salt_size.patch
Description:

Attachment: buffer_overflows.patch
Description: