Re: [PATCH][BZ #16072] Fix stack overflow due to large AF_INET6 requests

["Carlos O'Donell" <carlos at redhat dot com>]

On 10/23/2013 10:52 PM, Siddhesh Poyarekar wrote:
> On Wed, Oct 23, 2013 at 02:58:26PM -0400, Carlos O'Donell wrote:
>>> -    tmpbuf = extend_alloca (tmpbuf, tmpbuflen, 2 * tmpbuflen);		      \
>>> +    if (!malloc_tmpbuf && __libc_use_alloca (alloca_used + 2 * tmpbuflen))    \
>>
>> __glibc_likely? Who has huge /etc/hosts files?
>>
> 
> OK.
> 
>>> +      tmpbuf = extend_alloca_account (tmpbuf, tmpbuflen, 2 * tmpbuflen,	      \
>>> +				      alloca_used);			      \
>>> +    else								      \
>>> +      {									      \
>>> +	char *newp = realloc (malloc_tmpbuf ? tmpbuf : NULL,		      \
>>> +			      2 * tmpbuflen);				      \
>>
>> realloc has no limit making it possible to DoS the system.
>>
>> Should we consider a fixed size constant as the upper limit for this malloc?
>>
>> See:
>> https://sourceware.org/glibc/wiki/Style_and_Conventions#Alloca_vs._Malloc
>> "If the size of the buffer is directly or indirectly under user control, 
>> consider imposing a maximum to help make denial-of-service attacks more 
>> difficult. "
>>
> 
> This size is controlled by the size of the DNS response or the
> response generated from /etc/hosts.  Unless an attacker has control of
> the DNS server (or access to modify /etc/hosts), I don't see this to
> be a problem.

Sounds good to me. If the attacker has access to the DNS server at worst
we have malloc fail and return NULL, which means we return EAI_MEMORY
without making the allocation, and return via free_and_return.

I'm OK with this patch.

Cheers,
Carlos.