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

Message ID 871s76fn4f.fsf@oldenburg.str.redhat.com
State New
Headers show
Series
  • inet/tst-if_index-long: New test case for CVE-2018-19591 [BZ #23927]
Related show

Commit Message

Florian Weimer Nov. 27, 2018, 5:26 p.m.
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.

Comments

Carlos O'Donell Nov. 30, 2018, 3:44 a.m. | #1
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>

> 



-- 
Cheers,
Carlos.
Rafal Luzynski Nov. 30, 2018, 9:12 a.m. | #2
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
Florian Weimer Nov. 30, 2018, 9:28 a.m. | #3
* 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

Patch

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>