From patchwork Mon Aug 13 18:02:51 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 957108 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-95203-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="MeQoXEZ3"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41q3Vg27H5z9sBD for ; Tue, 14 Aug 2018 04:03:06 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:to:subject:mime-version:content-type :content-transfer-encoding:message-id:from; q=dns; s=default; b= ycuUmMfh/mljAIqP90kAqHqJqmOC5tFGiGPr48nglF3b9jlBCTHw4YdfZ9AyOIuQ mvXWj93Guv/fuH6SAAOeFokP6UqYFVTg3uBqHFIt3PUYgrGJO5VgmWYY6c6iyorU +MsVeK9sgIGVnpQ0PMTm81o0wZfdDX7o8qyUaEwt1gI= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:to:subject:mime-version:content-type :content-transfer-encoding:message-id:from; s=default; bh=Q2YA5G KhbS5NTjJUE49aQinaPsE=; b=MeQoXEZ3rjHzD7Auk2nKU3g962qevzOFjqSPXQ 92iPEcu//dkuJSPKXx/i1C71zu+HAyCPK2BX7THM62+2FDLEx0uFU+zz0MkB39Ku /550mFoVxssMqI2E9IsGZ5Cks+pNWVYzZR7LwzvKkCqoV2A767qXnkOLsJvvhAKQ fMb9U= Received: (qmail 37667 invoked by alias); 13 Aug 2018 18:03:00 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 37622 invoked by uid 89); 13 Aug 2018 18:02:55 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=cancellation X-HELO: mx1.redhat.com Date: Mon, 13 Aug 2018 20:02:51 +0200 To: libc-alpha@sourceware.org Subject: [PATCH] nss_files: Fix file stream leak in aliases lookup [BZ #23521] User-Agent: Heirloom mailx 12.5 7/5/10 MIME-Version: 1.0 Message-Id: <20180813180251.2894C40566487@oldenburg.str.redhat.com> From: fweimer@redhat.com (Florian Weimer) In order to get a clean test case, it was necessary to fix partially fixed bug 23522 as well. 2018-08-13 Florian Weimer [BZ #23521] [BZ #23522] * nss/nss_files/files-alias.c (get_next_alias): During :include: processing, bail out if no room, and close the stream before returning ERANGE. * nss/Makefile (tests): Add tst-nss-files-alias-leak. (tst-nss-files-alias-leak): Link with libdl. (tst-nss-files-alias-leak.out): Depend on nss_files. * nss/tst-nss-files-alias-leak.c: New file. diff --git a/nss/Makefile b/nss/Makefile index 66fac7f5b8..5209fc0456 100644 --- a/nss/Makefile +++ b/nss/Makefile @@ -65,6 +65,7 @@ ifeq (yes,$(build-shared)) tests += tst-nss-files-hosts-erange tests += tst-nss-files-hosts-multi tests += tst-nss-files-hosts-getent +tests += tst-nss-files-alias-leak endif # If we have a thread library then we can test cancellation against @@ -171,3 +172,5 @@ endif $(objpfx)tst-nss-files-hosts-erange: $(libdl) $(objpfx)tst-nss-files-hosts-multi: $(libdl) $(objpfx)tst-nss-files-hosts-getent: $(libdl) +$(objpfx)tst-nss-files-alias-leak: $(libdl) +$(objpfx)tst-nss-files-alias-leak.out: $(objpfx)/libnss_files.so diff --git a/nss/nss_files/files-alias.c b/nss/nss_files/files-alias.c index cfd34b66b9..35b0bfc5d2 100644 --- a/nss/nss_files/files-alias.c +++ b/nss/nss_files/files-alias.c @@ -221,6 +221,13 @@ get_next_alias (FILE *stream, const char *match, struct aliasent *result, { while (! feof_unlocked (listfile)) { + if (room_left < 2) + { + free (old_line); + fclose (listfile); + goto no_more_room; + } + first_unused[room_left - 1] = '\xff'; line = fgets_unlocked (first_unused, room_left, listfile); @@ -229,6 +236,7 @@ get_next_alias (FILE *stream, const char *match, struct aliasent *result, if (first_unused[room_left - 1] != '\xff') { free (old_line); + fclose (listfile); goto no_more_room; } @@ -256,6 +264,7 @@ get_next_alias (FILE *stream, const char *match, struct aliasent *result, + __alignof__ (char *))) { free (old_line); + fclose (listfile); goto no_more_room; } room_left -= ((first_unused - cp) diff --git a/nss/tst-nss-files-alias-leak.c b/nss/tst-nss-files-alias-leak.c new file mode 100644 index 0000000000..26d38e2dba --- /dev/null +++ b/nss/tst-nss-files-alias-leak.c @@ -0,0 +1,237 @@ +/* Check for file descriptor leak in alias :include: processing (bug 23521). + 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 + . */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +static struct support_chroot *chroot_env; + +/* Number of the aliases for the "many" user. This must be large + enough to trigger reallocation for the pointer array, but result in + answers below the maximum size tried in do_test. */ +enum { many_aliases = 30 }; + +static void +prepare (int argc, char **argv) +{ + chroot_env = support_chroot_create + ((struct support_chroot_configuration) { } ); + + char *path = xasprintf ("%s/etc/aliases", chroot_env->path_chroot); + add_temp_file (path); + support_write_file_string + (path, + "user1: :include:/etc/aliases.user1\n" + "user2: :include:/etc/aliases.user2\n" + "comment: comment1, :include:/etc/aliases.comment\n" + "many: :include:/etc/aliases.many\n"); + free (path); + + path = xasprintf ("%s/etc/aliases.user1", chroot_env->path_chroot); + add_temp_file (path); + support_write_file_string (path, "alias1\n"); + free (path); + + path = xasprintf ("%s/etc/aliases.user2", chroot_env->path_chroot); + add_temp_file (path); + support_write_file_string (path, "alias1a, alias2\n"); + free (path); + + path = xasprintf ("%s/etc/aliases.comment", chroot_env->path_chroot); + add_temp_file (path); + support_write_file_string + (path, + /* The line must be longer than the line with the :include: + directive in /etc/aliases. */ + "# Long line. ##############################################\n" + "comment2\n"); + free (path); + + path = xasprintf ("%s/etc/aliases.many", chroot_env->path_chroot); + add_temp_file (path); + FILE *fp = xfopen (path, "w"); + for (int i = 0; i < many_aliases; ++i) + fprintf (fp, "a%d\n", i); + TEST_VERIFY_EXIT (! ferror (fp)); + xfclose (fp); + free (path); +} + +/* The names of the users to test. */ +static const char *users[] = { "user1", "user2", "comment", "many" }; + +static void +check_aliases (int id, const struct aliasent *e) +{ + TEST_VERIFY_EXIT (id >= 0 || id < array_length (users)); + const char *name = users[id]; + TEST_COMPARE_BLOB (e->alias_name, strlen (e->alias_name), + name, strlen (name)); + + switch (id) + { + case 0: + TEST_COMPARE (e->alias_members_len, 1); + TEST_COMPARE_BLOB (e->alias_members[0], strlen (e->alias_members[0]), + "alias1", strlen ("alias1")); + break; + + case 1: + TEST_COMPARE (e->alias_members_len, 2); + TEST_COMPARE_BLOB (e->alias_members[0], strlen (e->alias_members[0]), + "alias1a", strlen ("alias1a")); + TEST_COMPARE_BLOB (e->alias_members[1], strlen (e->alias_members[1]), + "alias2", strlen ("alias2")); + break; + + case 2: + TEST_COMPARE (e->alias_members_len, 2); + TEST_COMPARE_BLOB (e->alias_members[0], strlen (e->alias_members[0]), + "comment1", strlen ("comment1")); + TEST_COMPARE_BLOB (e->alias_members[1], strlen (e->alias_members[1]), + "comment2", strlen ("comment2")); + break; + + case 3: + TEST_COMPARE (e->alias_members_len, many_aliases); + for (int i = 0; i < e->alias_members_len; ++i) + { + char alias[30]; + int len = snprintf (alias, sizeof (alias), "a%d", i); + TEST_VERIFY_EXIT (len > 0); + TEST_COMPARE_BLOB (e->alias_members[i], strlen (e->alias_members[i]), + alias, len); + } + break; + } +} + +static int +do_test (void) +{ + /* Make sure we don't try to load the module in the chroot. */ + if (dlopen (LIBNSS_FILES_SO, RTLD_NOW) == NULL) + FAIL_EXIT1 ("could not load " LIBNSS_FILES_SO ": %s", dlerror ()); + + /* Some of these descriptors will become unavailable if there is a + file descriptor leak. 10 is chosen somewhat arbitrarily. The + array must be longer than the number of files opened by nss_files + at the same time (currently that number is 2). */ + int next_descriptors[10]; + for (size_t i = 0; i < array_length (next_descriptors); ++i) + { + next_descriptors[i] = dup (0); + TEST_VERIFY_EXIT (next_descriptors[i] > 0); + } + for (size_t i = 0; i < array_length (next_descriptors); ++i) + xclose (next_descriptors[i]); + + support_become_root (); + if (!support_can_chroot ()) + return EXIT_UNSUPPORTED; + + __nss_configure_lookup ("aliases", "files"); + + xchroot (chroot_env->path_chroot); + + /* Attempt various buffer sizes. If the operation succeeds, we + expect correct data. */ + for (int id = 0; id < array_length (users); ++id) + { + bool found = false; + for (size_t size = 1; size <= 1000; ++size) + { + void *buffer = malloc (size); + struct aliasent result; + struct aliasent *res; + errno = EINVAL; + int ret = getaliasbyname_r (users[id], &result, buffer, size, &res); + if (ret == 0) + { + if (res != NULL) + { + found = true; + check_aliases (id, res); + } + else + { + support_record_failure (); + printf ("error: failed lookup for user \"%s\", size %zu\n", + users[id], size); + } + } + else if (ret != ERANGE) + { + support_record_failure (); + printf ("error: invalid return code %d (user \%s\", size %zu)\n", + ret, users[id], size); + } + free (buffer); + + /* Make sure that we did not have a file descriptor leak. */ + for (size_t i = 0; i < array_length (next_descriptors); ++i) + { + int new_fd = dup (0); + if (new_fd != next_descriptors[i]) + { + support_record_failure (); + printf ("error: descriptor %d at index %zu leaked" + " (user \"%s\", size %zu)\n", + next_descriptors[i], i, users[id], size); + + /* Close unexpected descriptor, the leak probing + descriptors, and the leaked descriptor + next_descriptors[i]. */ + xclose (new_fd); + for (size_t j = 0; j <= i; ++j) + xclose (next_descriptors[j]); + goto next_size; + } + } + for (size_t i = 0; i < array_length (next_descriptors); ++i) + xclose (next_descriptors[i]); + + next_size: + ; + } + if (!found) + { + support_record_failure (); + printf ("error: user %s not found\n", users[id]); + } + } + + support_chroot_free (chroot_env); + return 0; +} + +#define PREPARE prepare +#include