On 02/02/13 16:43, Gert Doering wrote:
> Hi,
>
> On Sat, Feb 02, 2013 at 03:36:37PM +0100, Arne Schwabe wrote:
>> The construct_name_value eventually call gc_malloc with NULL as gc which
>> will trigger an assertion
> [..]
>> - char *str = construct_name_value (name_tmp, val_tmp, NULL);
>> + char *str = construct_name_value (name_tmp, val_tmp, &gc);
>> if (platform_putenv(str))
>> {
>> msg (M_WARN | M_ERRNO, "putenv('%s') failed", str);
>
> NAK!
>
> platform_putenv() calls putenv() on non-windows platforms, and that one
> will just add a pointer to the string passed to it to envp[] (at least
> that's how I read the manpage: "The string pointed to by string becomes
> part of the environment, so altering the string changes the environment").Correct ... Please see this thread for more info (including proof-of-concept code) <http://thread.gmane.org/gmane.network.openvpn.devel/7090> > So your environment now points to a garbage buffer, which is then > destroyed again. > > I think there needs to be a strdup() here... Yeah, but that would be prone to leak memory again ... > (And I seem to remember that "some of the buffer functions" did exactly > this when &gc was NULL, but maybe we threw this out last year... David?) I think the commit you think about is this one: commit bee92b479414d12035b0422f81ac5fcfe14fa645 Author: Adriaan de Jong <dej...@fox-it.com> List-Post: openvpn-devel@lists.sourceforge.net Date: Sun Feb 5 12:51:25 2012 +0100 Removed support for calling gc_malloc with a NULL gc_arena struct Signed-off-by: Adriaan de Jong <dej...@fox-it.com> Acked-by: David Sommerseth <dav...@redhat.com> Signed-off-by: David Sommerseth <dav...@redhat.com> That basically impacted all calls ending up in gc_malloc(). But this was prone to leak memory when called with NULL as the gc_arena pointer. After a discussion, we ended up agreeing to solve issues this patch would flush open - instead of trying to fix those leaking areas instantly; then it would just have been a temporary fix until it would be abused again. -- kind regards, David Sommerseth
signature.asc
Description: OpenPGP digital signature
