From patchwork Wed Dec 16 14:21:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddhesh Poyarekar X-Patchwork-Id: 41464 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9BF6E38708E1; Wed, 16 Dec 2020 14:22:14 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9BF6E38708E1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1608128534; bh=CxOj4Zywe8RRBE8l0el3ateQ7DDMHu+Y0aMbUvBkDxY=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=OjYDgAXRqGtXn7pML7Fhzto2/6c4JTRfwMnancedNIPHj6o2rg+Kk3NgpO/H3BO0p FpUBT/uYFmPM/jUwNv+s+WkSeNkFr8bIOhGRaa1Pp1MjC9Kw1w3U93jaD9VBaDB/dA ZpRpEvxZX9FwDILhszRm7aZTRzU7KRIozSNir1+o= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from quail.birch.relay.mailchannels.net (quail.birch.relay.mailchannels.net [23.83.209.151]) by sourceware.org (Postfix) with ESMTPS id 7982A386F022 for ; Wed, 16 Dec 2020 14:22:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 7982A386F022 X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 41DB16428D7; Wed, 16 Dec 2020 14:22:05 +0000 (UTC) Received: from pdx1-sub0-mail-a30.g.dreamhost.com (100-98-118-97.trex.outbound.svc.cluster.local [100.98.118.97]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id E17EE64204F; Wed, 16 Dec 2020 14:22:02 +0000 (UTC) X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from pdx1-sub0-mail-a30.g.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384) by 0.0.0.0:2500 (trex/5.18.11); Wed, 16 Dec 2020 14:22:05 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Left-Robust: 026392f93bf75102_1608128525066_2664071390 X-MC-Loop-Signature: 1608128525066:3719289953 X-MC-Ingress-Time: 1608128525066 Received: from pdx1-sub0-mail-a30.g.dreamhost.com (localhost [127.0.0.1]) by pdx1-sub0-mail-a30.g.dreamhost.com (Postfix) with ESMTP id 73E407EEDD; Wed, 16 Dec 2020 06:22:02 -0800 (PST) Received: from rhbox.redhat.com (unknown [1.186.101.110]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a30.g.dreamhost.com (Postfix) with ESMTPSA id B5CD97EED8; Wed, 16 Dec 2020 06:22:00 -0800 (PST) X-DH-BACKEND: pdx1-sub0-mail-a30 To: libc-alpha@sourceware.org Subject: [PATCH] addmntent: Remove unbounded alloca usage from getmntent [BZ#27083] Date: Wed, 16 Dec 2020 19:51:43 +0530 Message-Id: <20201216142143.173716-1-siddhesh@sourceware.org> X-Mailer: git-send-email 2.29.2 MIME-Version: 1.0 X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NEUTRAL, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Siddhesh Poyarekar via Libc-alpha From: Siddhesh Poyarekar Reply-To: Siddhesh Poyarekar Cc: fweimer@redhat.com Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" The addmntent function replicates elements of struct mnt on stack using alloca, which is unsafe. Use malloc instead. Also add a test to check all escaped characters with addmntent and getmntent. --- misc/Makefile | 2 +- misc/mntent_r.c | 128 ++++++++++++++++++--------------------- misc/tst-mntent-escape.c | 101 ++++++++++++++++++++++++++++++ 3 files changed, 160 insertions(+), 71 deletions(-) create mode 100644 misc/tst-mntent-escape.c diff --git a/misc/Makefile b/misc/Makefile index 58959f6913..92816af2a2 100644 --- a/misc/Makefile +++ b/misc/Makefile @@ -88,7 +88,7 @@ tests := tst-dirname tst-tsearch tst-fdset tst-mntent tst-hsearch \ tst-preadvwritev tst-preadvwritev64 tst-makedev tst-empty \ tst-preadvwritev2 tst-preadvwritev64v2 tst-warn-wide \ tst-ldbl-warn tst-ldbl-error tst-dbl-efgcvt tst-ldbl-efgcvt \ - tst-mntent-autofs tst-syscalls + tst-mntent-autofs tst-syscalls tst-mntent-escape # Tests which need libdl. ifeq (yes,$(build-shared)) diff --git a/misc/mntent_r.c b/misc/mntent_r.c index 0e8f10007e..b50ba0de89 100644 --- a/misc/mntent_r.c +++ b/misc/mntent_r.c @@ -23,6 +23,7 @@ #include #include #include +#include #define flockfile(s) _IO_flockfile (s) #define funlockfile(s) _IO_funlockfile (s) @@ -212,87 +213,74 @@ __getmntent_r (FILE *stream, struct mntent *mp, char *buffer, int bufsiz) libc_hidden_def (__getmntent_r) weak_alias (__getmntent_r, getmntent_r) +/* Encode whitespaces and backslashes into their octal form. */ +static char * +encode_name (const char *name) +{ + const char *rp = name; + char *ret; + char c; + const char *encode_chars = " \t\n\\"; + + /* In the worst case the length of the string can increase to + four times the current length. */ + char *wp = ret = (char *) malloc (strlen (name) * 4 + 1); -/* We have to use an encoding for names if they contain spaces or tabs. - To be able to represent all characters we also have to escape the - backslash itself. This "function" must be a macro since we use - `alloca'. */ -#define encode_name(name) \ - do { \ - const char *rp = name; \ - \ - while (*rp != '\0') \ - if (*rp == ' ' || *rp == '\t' || *rp == '\n' || *rp == '\\') \ - break; \ - else \ - ++rp; \ - \ - if (*rp != '\0') \ - { \ - /* In the worst case the length of the string can increase to \ - four times the current length. */ \ - char *wp; \ - \ - rp = name; \ - name = wp = (char *) alloca (strlen (name) * 4 + 1); \ - \ - do \ - if (*rp == ' ') \ - { \ - *wp++ = '\\'; \ - *wp++ = '0'; \ - *wp++ = '4'; \ - *wp++ = '0'; \ - } \ - else if (*rp == '\t') \ - { \ - *wp++ = '\\'; \ - *wp++ = '0'; \ - *wp++ = '1'; \ - *wp++ = '1'; \ - } \ - else if (*rp == '\n') \ - { \ - *wp++ = '\\'; \ - *wp++ = '0'; \ - *wp++ = '1'; \ - *wp++ = '2'; \ - } \ - else if (*rp == '\\') \ - { \ - *wp++ = '\\'; \ - *wp++ = '\\'; \ - } \ - else \ - *wp++ = *rp; \ - while (*rp++ != '\0'); \ - } \ - } while (0) + if (wp == NULL) + return NULL; + while ((c = *rp++) != '\0') + { + if (strchr (encode_chars, c) == NULL) + *wp++ = c; + else + { + *wp++ = '\\'; + *wp++ = ((c & 0xc0) >> 6) + '0'; + *wp++ = ((c & 0x38) >> 3) + '0'; + *wp++ = ((c & 0x07) >> 0) + '0'; + } + } + *wp = '\0'; + return ret; +} /* Write the mount table entry described by MNT to STREAM. Return zero on success, nonzero on failure. */ int __addmntent (FILE *stream, const struct mntent *mnt) { - struct mntent mntcopy = *mnt; + char *mnt_fsname = NULL, *mnt_dir = NULL, *mnt_type = NULL, *mnt_opts = NULL; + int ret = 1; + if (fseek (stream, 0, SEEK_END)) - return 1; + return ret; /* Encode spaces and tabs in the names. */ - encode_name (mntcopy.mnt_fsname); - encode_name (mntcopy.mnt_dir); - encode_name (mntcopy.mnt_type); - encode_name (mntcopy.mnt_opts); - - return (fprintf (stream, "%s %s %s %s %d %d\n", - mntcopy.mnt_fsname, - mntcopy.mnt_dir, - mntcopy.mnt_type, - mntcopy.mnt_opts, - mntcopy.mnt_freq, - mntcopy.mnt_passno) < 0 - || fflush (stream) != 0); + mnt_fsname = encode_name (mnt->mnt_fsname); + if (mnt_fsname == NULL) + goto out; + mnt_dir = encode_name (mnt->mnt_dir); + if (mnt_dir== NULL) + goto out; + mnt_type = encode_name (mnt->mnt_type); + if (mnt_type == NULL) + goto out; + mnt_opts = encode_name (mnt->mnt_opts); + if (mnt_opts == NULL) + goto out; + + ret = (fprintf (stream, "%s %s %s %s %d %d\n", + mnt_fsname, mnt_dir, mnt_type, mnt_opts, + mnt->mnt_freq, mnt->mnt_passno) < 0 + || fflush (stream) != 0); +out: + free (mnt_fsname); + free (mnt_dir); + free (mnt_type); + free (mnt_opts); + + return ret; } weak_alias (__addmntent, addmntent) diff --git a/misc/tst-mntent-escape.c b/misc/tst-mntent-escape.c new file mode 100644 index 0000000000..c1db428a9d --- /dev/null +++ b/misc/tst-mntent-escape.c @@ -0,0 +1,101 @@ +/* Test mntent interface with escaped sequences. + Copyright (C) 2020 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 + +struct const_mntent +{ + const char *mnt_fsname; + const char *mnt_dir; + const char *mnt_type; + const char *mnt_opts; + int mnt_freq; + int mnt_passno; + const char *expected; +}; + +struct const_mntent tests[] = +{ + {"/dev/hda1", "/some dir", "ext2", "defaults", 1, 2, + "/dev/hda1 /some\\040dir ext2 defaults 1 2\n"}, + {"device name", "/some dir", "tmpfs", "defaults", 1, 2, + "device\\040name /some\\040dir tmpfs defaults 1 2\n"}, + {" ", "/some dir", "tmpfs", "defaults", 1, 2, + "\\040 /some\\040dir tmpfs defaults 1 2\n"}, + {"\t", "/some dir", "tmpfs", "defaults", 1, 2, + "\\011 /some\\040dir tmpfs defaults 1 2\n"}, + {"\\", "/some dir", "tmpfs", "defaults", 1, 2, + "\\134 /some\\040dir tmpfs defaults 1 2\n"}, +}; + +static int +do_test (void) +{ + for (int i = 0; i < sizeof (tests) / sizeof (struct const_mntent); i++) + { + char buf[128]; + struct mntent *ret, curtest; + FILE *fp = fmemopen (buf, sizeof (buf), "w+"); + + if (fp == NULL) + { + printf ("Failed to open file\n"); + return 1; + } + + curtest.mnt_fsname = strdupa (tests[i].mnt_fsname); + curtest.mnt_dir = strdupa (tests[i].mnt_dir); + curtest.mnt_type = strdupa (tests[i].mnt_type); + curtest.mnt_opts = strdupa (tests[i].mnt_opts); + curtest.mnt_freq = tests[i].mnt_freq; + curtest.mnt_passno = tests[i].mnt_passno; + + if (addmntent (fp, &curtest) != 0) + { + support_record_failure (); + continue; + } + + TEST_COMPARE_STRING (buf, tests[i].expected); + + rewind (fp); + ret = getmntent (fp); + if (ret == NULL) + { + support_record_failure (); + continue; + } + + TEST_COMPARE_STRING(tests[i].mnt_fsname, ret->mnt_fsname); + TEST_COMPARE_STRING(tests[i].mnt_dir, ret->mnt_dir); + TEST_COMPARE_STRING(tests[i].mnt_type, ret->mnt_type); + TEST_COMPARE_STRING(tests[i].mnt_opts, ret->mnt_opts); + TEST_COMPARE(tests[i].mnt_freq, ret->mnt_freq); + TEST_COMPARE(tests[i].mnt_passno, ret->mnt_passno); + + fclose (fp); + } + + return 0; +} + +#include