inet/tst-if_index-long: New test case for CVE-2018-19591 [BZ #23927]

  2018-11-27  Florian Weimer  <fweimer@redhat.com>

	[BZ #23927]
	CVE-2018-19591
	* inet/tst-if_index-long.c: New file.
	* inet/Makefile (tests): Add tst-if_index-long.
  

On 11/27/18 12:26 PM, Florian Weimer wrote:
> 2018-11-27  Florian Weimer  <fweimer@redhat.com>
> 
> 	[BZ #23927]
> 	CVE-2018-19591
> 	* inet/tst-if_index-long.c: New file.
> 	* inet/Makefile (tests): Add tst-if_index-long.

LGTM (pending review of the required support infrastructure).

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> diff --git a/inet/Makefile b/inet/Makefile
> index 09f5ba78fc..7782913b4c 100644
> --- a/inet/Makefile
> +++ b/inet/Makefile
> @@ -52,7 +52,7 @@ aux := check_pf check_native ifreq
>  tests := htontest test_ifindex tst-ntoa tst-ether_aton tst-network \
>  	 tst-gethnm test-ifaddrs bug-if1 test-inet6_opt tst-ether_line \
>  	 tst-getni1 tst-getni2 tst-inet6_rth tst-checks tst-checks-posix \
> -	 tst-sockaddr test-hnto-types
> +	 tst-sockaddr test-hnto-types tst-if_index-long
>  
>  # tst-deadline must be linked statically so that we can access
>  # internal functions.
> diff --git a/inet/tst-if_index-long.c b/inet/tst-if_index-long.c
> new file mode 100644
> index 0000000000..3dc74874e5
> --- /dev/null
> +++ b/inet/tst-if_index-long.c
> @@ -0,0 +1,61 @@
> +/* Check for descriptor leak in if_nametoindex with a long interface name.
> +   Copyright (C) 2018 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +/* This test checks for a descriptor leak in case of a long interface
> +   name (CVE-2018-19591, bug 23927).  */
> +
> +#include <errno.h>
> +#include <net/if.h>
> +#include <netdb.h>
> +#include <string.h>
> +#include <support/check.h>
> +#include <support/descriptors.h>
> +#include <support/support.h>
> +
> +static int
> +do_test (void)
> +{
> +  struct support_descriptors *descrs = support_descriptors_list ();
> +
> +  /* Prepare a name which is just as long as required for trigging the
> +     bug.  */
> +  char name[IFNAMSIZ + 1];
> +  memset (name, 'A', IFNAMSIZ);
> +  name[IFNAMSIZ] = '\0';
> +  TEST_COMPARE (strlen (name), IFNAMSIZ);
> +  struct ifreq ifr;
> +  TEST_COMPARE (strlen (name), sizeof (ifr.ifr_name));
> +
> +  /* Test directly via if_nametoindex.  */
> +  TEST_COMPARE (if_nametoindex (name), 0);
> +  TEST_COMPARE (errno, ENODEV);
> +  support_descriptors_check (descrs);
> +
> +  /* Same test via getaddrinfo.  */
> +  char *host = xasprintf ("fea0::%%%s", name);
> +  struct addrinfo hints = { .ai_flags = AI_NUMERICHOST, };
> +  struct addrinfo *ai;
> +  TEST_COMPARE (getaddrinfo (host, NULL, &hints, &ai), EAI_NONAME);
> +  support_descriptors_check (descrs);
> +
> +  support_descriptors_free (descrs);
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
>
  

27.11.2018 18:26 Florian Weimer <fweimer@redhat.com> wrote:
> [...]
> diff --git a/inet/tst-if_index-long.c b/inet/tst-if_index-long.c
> new file mode 100644
> index 0000000000..3dc74874e5
> --- /dev/null
> +++ b/inet/tst-if_index-long.c
> @@ -0,0 +1,61 @@
> +/* Check for descriptor leak in if_nametoindex with a long interface
> name.
> +   Copyright (C) 2018 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +/* This test checks for a descriptor leak in case of a long interface
> +   name (CVE-2018-19591, bug 23927).  */

These two lines look almost the same as the first line of the file.
Are you sure you need this information repeated twice?

My review is very quick, I can only confirm that your patch applies,
builds and runs fine on x86_64, also it correctly fails if the commit
d527c86 [1] is removed.

Regards,

Rafal

[1]
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=d527c860f5a3f0ed687bd03f0cb464612dc23408
  

* Rafal Luzynski:

> 27.11.2018 18:26 Florian Weimer <fweimer@redhat.com> wrote:
>> [...]
>> diff --git a/inet/tst-if_index-long.c b/inet/tst-if_index-long.c
>> new file mode 100644
>> index 0000000000..3dc74874e5
>> --- /dev/null
>> +++ b/inet/tst-if_index-long.c
>> @@ -0,0 +1,61 @@
>> +/* Check for descriptor leak in if_nametoindex with a long interface
>> name.
>> +   Copyright (C) 2018 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <http://www.gnu.org/licenses/>.  */
>> +
>> +/* This test checks for a descriptor leak in case of a long interface
>> +   name (CVE-2018-19591, bug 23927).  */
>
> These two lines look almost the same as the first line of the file.
> Are you sure you need this information repeated twice?

I think it's useful to reference the bug number somewhere in the test
case and stay within the length limit for the first line of the file.

Thanks,
Florian