[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <201209151922.11583.geissert@debian.org>
Date: Sat, 15 Sep 2012 19:22:06 -0500
From: Raphael Geissert <geissert@...ian.org>
To: oss-security@...ts.openwall.com
Subject: Re: CVE request - mcrypt buffer overflow flaw
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@...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.
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
View attachment "uninitialized_salt_size.patch" of type "text/x-patch" (363 bytes)
View attachment "buffer_overflows.patch" of type "text/x-patch" (784 bytes)
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
