Re: [libmicrohttpd] Issue on receiving very long URI

[Evgeny Grin]

Hello Shikha,

Looks like something wrong with MHD sources on your computer.
Please download the latest sources archive or checkout the latest git 
master.

I see that the quoted code for 'try_grow_read_buffer()' was taken from 
MHD version 0.9.65 or later while code for 'MHD_pool_reallocate()' 
matches code for version 0.9.64 (or earlier version).

Function 'MHD_pool_reallocate()' was fixed in v0.9.65. You cannot mix 
source files from different MHD version.

--
Best Wishes,
Evgeny

19.08.2020 10:25, Shikha Sharma:
> Hi
>
> I have observed a crash in MHD after using very long URI. In function
>
> int
> try_grow_read_buffer (struct MHD_Connection *connection)
> {
>      size_t new_size;
>    size_t avail_size;
>
>    avail_size = MHD_pool_get_free (connection->pool);
>    if (0 == avail_size)
>      return false; /* No more space available */
>    if (0 == connection->read_buffer_size)
>      new_size = avail_size / 2; /* Use half of available buffer for 
> reading */
>    else
>      {
>        size_t grow_size;
>
>        grow_size = avail_size / 8;
>        if (MHD_BUF_INC_SIZE > grow_size)
>        { /* Shortage of space */
>            /* Shortage of space, but grow is mandatory */
>            static const size_t small_inc = MHD_BUF_INC_SIZE / 8;
>            if (small_inc < avail_size)
>                grow_size = small_inc;
>            else
>                grow_size = avail_size;
>        }
>        new_size = connection->read_buffer_size + grow_size;
>      }
>    /* we can actually grow the buffer, do it! */
>    connection->read_buffer = MHD_pool_reallocate (connection->pool,
> connection->read_buffer,
> connection->read_buffer_size,
>                                                   new_size);
>    mhd_assert (NULL != connection->read_buffer);
>    connection->read_buffer_size = new_size;
>    return MHD_YES;
> }
>
>
> In my scenario, MHD_pool_reallocate returns NULL as it can not 
> reallocate the memory. Thus connection->read_buffer is set to NULL and 
> the function returns MHD_YES as if it was able to grow the buffer.
>
> The next access to connection->read_buffer leads to crash.
>
>
> Debug trace for MHD_pool_reallocate returning NULL :
>
> Breakpoint 2, MHD_pool_reallocate (pool=0x7fffa0008d90, 
> old=0x7fffa0000c70, old_size=23164, new_size=new_size@entry=24364) at 
> memorypool.c:252
> 252    {
> (gdb) p *pool
> $88 = {memory = 0x7fffa0000c70 "GET 
> /q/init-req/val1/callingParty/316543216/hostid/bfx1/SessionId/123/calledParty/1415926535897932384626433832795028841971693993751058209749445923078164062862089986280348253421170679821480865132823066"..., 
> size = 32768, pos = 23168, end = 32768, is_mmap = 0}
> (gdb) n
> 256      asize = ROUND_TO_ALIGN (new_size);
> (gdb)
> 257      if ( (0 == asize) &&
> (gdb)
> 260      if ( (pool->end < old_size) ||
> (gdb)
> 264      if ( (pool->pos >= old_size) &&
> (gdb)
> 265           (&pool->memory[pool->pos - old_size] == old) )
> (gdb)
> 264      if ( (pool->pos >= old_size) &&
> (gdb)
> 281      if (asize <= old_size)
> (gdb)
> 283      if ((pool->pos + asize >= pool->pos) &&
> (gdb)
> 297    }
> (gdb)
> 259        return NULL; /* new_size too close to SIZE_MAX */
> (gdb)
> 297    }
>
> The functionality of try_grow_read_buffer was changed and the snippet 
> above is latest. In the old functionality:
>
> buf = MHD_pool_reallocate (connection->pool,
>                               connection->read_buffer,
>                               connection->read_buffer_size,
>                               new_size);
>    if (NULL == buf)
>      return MHD_NO;
>    /* we can actually grow the buffer, do it! */
>    connection->read_buffer = buf;
>    connection->read_buffer_size = new_size;
>    return MHD_YES;
>
>
> which returns MHD_NO in case reallocation fails.
>
> Thanks & Regards,
>
> Shikha