From patchwork Tue Dec 22 11:51:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddhesh Poyarekar X-Patchwork-Id: 41513 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 AAFCE3836C0F; Tue, 22 Dec 2020 11:51:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AAFCE3836C0F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1608637919; bh=ya8g3kYsz4Yi7e9EL1LEZgcQ60CPKh/LJFq3yHIghBI=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=AyjqtWRbf+NfiWRAbO59xhTkL6CIB3fJvZl4NcE6MvZx77sfFiU3kDhzMYeh+q8LV SvcolCuLdBQoxdA0thaCkkskZLvem5SvOwMFN6D9eorKBAvhnkdY96L+k6Rt7Plyez eWFxNzEOWfqtWsjbSjUsN6vhom27bmzZWjxIzqaM= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from dog.birch.relay.mailchannels.net (dog.birch.relay.mailchannels.net [23.83.209.48]) by sourceware.org (Postfix) with ESMTPS id 551DD3836C03 for ; Tue, 22 Dec 2020 11:51:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 551DD3836C03 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 BB6AB342711; Tue, 22 Dec 2020 11:51:54 +0000 (UTC) Received: from pdx1-sub0-mail-a49.g.dreamhost.com (100-96-9-178.trex.outbound.svc.cluster.local [100.96.9.178]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 4593E342182; Tue, 22 Dec 2020 11:51:54 +0000 (UTC) X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from pdx1-sub0-mail-a49.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); Tue, 22 Dec 2020 11:51:54 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Glossy-Imminent: 3fd0656c434ed3ec_1608637914573_3801438131 X-MC-Loop-Signature: 1608637914573:1863933528 X-MC-Ingress-Time: 1608637914573 Received: from pdx1-sub0-mail-a49.g.dreamhost.com (localhost [127.0.0.1]) by pdx1-sub0-mail-a49.g.dreamhost.com (Postfix) with ESMTP id 065758AB4D; Tue, 22 Dec 2020 03:51:53 -0800 (PST) Received: from rhbox.intra.reserved-bit.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-a49.g.dreamhost.com (Postfix) with ESMTPSA id E48537FBB8; Tue, 22 Dec 2020 03:51:50 -0800 (PST) X-DH-BACKEND: pdx1-sub0-mail-a49 To: libc-alpha@sourceware.org Subject: [PATCH v3] addmntent: Remove unbounded alloca usage from getmntent [BZ#27083] Date: Tue, 22 Dec 2020 17:21:40 +0530 Message-Id: <20201222115140.2055339-1-siddhesh@sourceware.org> X-Mailer: git-send-email 2.29.2 MIME-Version: 1.0 X-Spam-Status: No, score=-9.1 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. Put characters directly into the stream, escaping them as they're being written out. Also add a test to check all escaped characters with addmntent and getmntent. --- Changes since last version: - Check for stream error only once, before flushing. misc/Makefile | 2 +- misc/mntent_r.c | 111 ++++++++++++++------------------------- misc/tst-mntent-escape.c | 101 +++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 74 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 90a73f2dda..c9148c5b5e 100644 --- a/misc/mntent_r.c +++ b/misc/mntent_r.c @@ -232,87 +232,52 @@ __getmntent_r (FILE *stream, struct mntent *mp, char *buffer, int bufsiz) libc_hidden_def (__getmntent_r) weak_alias (__getmntent_r, getmntent_r) +/* Write STR into STREAM, escaping whitespaces as we go. Do not check for + errors here; we check the stream status in __ADDMNTENT. */ +static void +write_string (FILE *stream, const char *str) +{ + char c; + const char *encode_chars = " \t\n\\"; -/* 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) - + while ((c = *str++) != '\0') + { + if (strchr (encode_chars, c) == NULL) + fputc_unlocked (c, stream); + else + { + fputc_unlocked ('\\', stream); + fputc_unlocked (((c & 0xc0) >> 6) + '0', stream); + fputc_unlocked (((c & 0x38) >> 3) + '0', stream); + fputc_unlocked (((c & 0x07) >> 0) + '0', stream); + } + } + fputc_unlocked (' ', stream); +} /* 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; + int ret = 1; + if (fseek (stream, 0, SEEK_END)) - return 1; - - /* 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); + return ret; + + flockfile (stream); + + write_string (stream, mnt->mnt_fsname); + write_string (stream, mnt->mnt_dir); + write_string (stream, mnt->mnt_type); + write_string (stream, mnt->mnt_opts); + fprintf (stream, "%d %d\n", mnt->mnt_freq, mnt->mnt_passno); + + ret = ferror (stream) != 0 || fflush (stream) != 0; + + funlockfile (stream); + + 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