[PATCH] Fix leak in getaddrinfo introduced by the fix for CVE-2023-4806.

[Siddhesh Poyarekar]

On 2023-09-22 18:11, Romain Geissler wrote:
> This patch fixes a very recently added leak in getaddrinfo (which was
> backported on release branches too).
> 
> I didn't spend much more than 5 minutes on investigating the code to end
> up with this patch, so it may be wrong. Quickly testing it on my side,
> it seems to work for me, but it definitely needs review from people who
> actually know this part of the code ;)
> 

Nice catch, thank you for noticing.

> Running a stripped down version of the newly added test
> nss/nss_test_gai_hv2_canonname.c with valgrind results in exposure of
> the leak:
> 
>> cat test.c
> 
> int main()
> {
>      char aHostName[256];
>      gethostname(aHostName,255);
> 
>      struct addrinfo hints = {};
>      struct addrinfo *result = NULL;
> 
>      hints.ai_family = AF_INET6;
>      hints.ai_flags = AI_ALL | AI_V4MAPPED | AI_CANONNAME;
> 
>      int ret = getaddrinfo(aHostName, NULL, &hints, &result);
> 
>      if (ret != 0)
>          return 1;
>      freeaddrinfo(result);
>      return 0;
> }
> 
>> /opt/1A/toolchain/x86_64-v19/bin/gcc -g -o test test.c
>> /opt/1A/toolchain/x86_64-v19/build-pack/default/bin/valgrind --leak-check=full ./test
>     ... (snapped)
> ==68017== 37 bytes in 1 blocks are definitely lost in loss record 1 of 1
> ==68017==    at 0x4840745: malloc (in /remote/tools/Linux/2.6/1A/toolchain/x86_64-v19.0.81/build-pack/19.0.81.0/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==68017==    by 0x48E7CDA: strdup (in /remote/tools/Linux/2.6/1A/toolchain/x86_64-v19.0.81/lib/libc.so.6)
> ==68017==    by 0x4936582: convert_hostent_to_gaih_addrtuple.isra.0 (in /remote/tools/Linux/2.6/1A/toolchain/x86_64-v19.0.81/lib/libc.so.6)
> ==68017==    by 0x4936787: gethosts (in /remote/tools/Linux/2.6/1A/toolchain/x86_64-v19.0.81/lib/libc.so.6)
> ==68017==    by 0x4938F37: getaddrinfo (in /remote/tools/Linux/2.6/1A/toolchain/x86_64-v19.0.81/lib/libc.so.6)
> ==68017==    by 0x4011A5: main (test.c:17)
>     ... (snapped)
> ---
>   sysdeps/posix/getaddrinfo.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
> index b4e8ea3880a..5f5bc3fd51f 100644
> --- a/sysdeps/posix/getaddrinfo.c
> +++ b/sysdeps/posix/getaddrinfo.c
> @@ -1199,6 +1199,7 @@ free_and_return:
>     if (res.free_at)
>       free (res.at);
>     free (res.canon);
> +  free (res.h_name);

Could you please consolidate all of this into a gaih_result_reset (&res) 
call?  There's an additional memset, but that should be negligible 
overhead for a cleaner abstraction.

Thanks,
Sid