Re: [PATCH 1/4] nscd: Stack-based buffer overflow in netgroup cache (bug 31677)

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 2024-04-24 12:08, Florian Weimer wrote:
> Using alloca matches what other caches do.  The request length is
> bounded by MAXKEYLEN.
> ---
>   nscd/netgroupcache.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c
> index 0c6e46f15c..24fbac7668 100644
> --- a/nscd/netgroupcache.c
> +++ b/nscd/netgroupcache.c
> @@ -502,12 +502,11 @@ addinnetgrX (struct database_dyn *db, int fd, request_header *req,
>         = (struct indataset *) mempool_alloc (db,
>   					    sizeof (*dataset) + req->key_len,
>   					    1);
> -  struct indataset dataset_mem;
>     bool cacheable = true;
>     if (__glibc_unlikely (dataset == NULL))
>       {
>         cacheable = false;

Can you please add a comment here stating that KEY_LEN is bounded by 
MAXKEYLEN?  Looks OK otherwise.

> -      dataset = &dataset_mem;
> +      dataset = alloca (sizeof (*dataset) + req->key_len);
>       }
>   
>     datahead_init_pos (&dataset->head, sizeof (*dataset) + req->key_len,