Re: [PATCH v3] getaddrinfo: Fix use after free in getcanonname (CVE-2023-4806)

[Florian Weimer <fweimer@redhat.com>]

* Siddhesh Poyarekar:

> diff --git a/nss/tst-nss-gai-hv2-canonname.root/postclean.req b/nss/tst-nss-gai-hv2-canonname.root/postclean.req
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/nss/tst-nss-gai-hv2-canonname.root/tst-nss-gai-hv2-canonname.script b/nss/tst-nss-gai-hv2-canonname.root/tst-nss-gai-hv2-canonname.script
> new file mode 100644
> index 0000000000..31848b4a28
> --- /dev/null
> +++ b/nss/tst-nss-gai-hv2-canonname.root/tst-nss-gai-hv2-canonname.script
> @@ -0,0 +1,2 @@
> +cp $B/nss/libnss_test_gai_hv2_canonname.so $L/libnss_test_gai_hv2_canonname.so.2
> +su
> diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c

> @@ -238,6 +239,15 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, int family,
>    res->at = array;
>    res->free_at = true;
>  
> +  /* Duplicate h_name because it may get reclaimed when the underlying storage
> +     is freed.  */
> +  if (res->hname == NULL)
> +    {
> +      res->hname = __strdup (h->h_name);
> +      if (res->hname == NULL)
> +	return false;
> +    }
> +
>    /* Update the next pointers on reallocation.  */
>    for (size_t i = 0; i < old; i++)
>      array[i].next = array + i + 1;
> @@ -262,7 +272,6 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, int family,
>  	}
>        array[i].next = array + i + 1;
>      }
> -  array[0].name = h->h_name;
>    array[count - 1].next = NULL;

I would expect

  array[0].name = res->h_name;

here, but that's not needed because the assignment happens in
generate_addrinfo, from res->canon, which may have been set with
res->h_name as a fallback?

Thanks,
Florian