From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from cyan.elm.relay.mailchannels.net (cyan.elm.relay.mailchannels.net [23.83.212.47]) by sourceware.org (Postfix) with ESMTPS id 137FC3870886 for ; Tue, 22 Dec 2020 10:08:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 137FC3870886 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 6F71B100653; Tue, 22 Dec 2020 10:08:57 +0000 (UTC) Received: from pdx1-sub0-mail-a49.g.dreamhost.com (100-96-5-6.trex.outbound.svc.cluster.local [100.96.5.6]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 69871100CB4; Tue, 22 Dec 2020 10:08:56 +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 10:08:57 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Desert-Troubled: 638aafd30f503193_1608631736717_3106741083 X-MC-Loop-Signature: 1608631736717:1667332942 X-MC-Ingress-Time: 1608631736716 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 1D9917FBB3; Tue, 22 Dec 2020 02:08:56 -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 3E9887FBB8; Tue, 22 Dec 2020 02:08:53 -0800 (PST) X-DH-BACKEND: pdx1-sub0-mail-a49 From: Siddhesh Poyarekar To: libc-alpha@sourceware.org Cc: fweimer@redhat.com Subject: [PATCH v2] addmntent: Remove unbounded alloca usage from getmntent [BZ#27083] Date: Tue, 22 Dec 2020 15:38:46 +0530 Message-Id: <20201222100846.2044093-1-siddhesh@sourceware.org> X-Mailer: git-send-email 2.29.2 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable 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-List-Received-Date: Tue, 22 Dec 2020 10:09:01 -0000 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. --- misc/Makefile | 2 +- misc/mntent_r.c | 113 ++++++++++++++------------------------- misc/tst-mntent-escape.c | 101 ++++++++++++++++++++++++++++++++++ 3 files changed, 142 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 :=3D 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 =20 # Tests which need libdl. ifeq (yes,$(build-shared)) diff --git a/misc/mntent_r.c b/misc/mntent_r.c index 90a73f2dda..ac5a83d1cf 100644 --- a/misc/mntent_r.c +++ b/misc/mntent_r.c @@ -232,87 +232,54 @@ __getmntent_r (FILE *stream, struct mntent *mp, cha= r *buffer, int bufsiz) libc_hidden_def (__getmntent_r) weak_alias (__getmntent_r, getmntent_r) =20 +/* Write STR into STREAM, escaping whitespaces as we go. */ +#define PUTC_OR_FAIL(_c, _st) if (fputc_unlocked ((_c), (_st)) < 0) retu= rn -1 +static int +write_string (FILE *stream, const char *str) +{ + char c; + const char *encode_chars =3D " \t\n\\"; =20 -/* 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 =3D name; \ - \ - while (*rp !=3D '\0') \ - if (*rp =3D=3D ' ' || *rp =3D=3D '\t' || *rp =3D=3D '\n' || *rp =3D= =3D '\\') \ - break; \ - else \ - ++rp; \ - \ - if (*rp !=3D '\0') \ - { \ - /* In the worst case the length of the string can increase to \ - four times the current length. */ \ - char *wp; \ - \ - rp =3D name; \ - name =3D wp =3D (char *) alloca (strlen (name) * 4 + 1); \ - \ - do \ - if (*rp =3D=3D ' ') \ - { \ - *wp++ =3D '\\'; \ - *wp++ =3D '0'; \ - *wp++ =3D '4'; \ - *wp++ =3D '0'; \ - } \ - else if (*rp =3D=3D '\t') \ - { \ - *wp++ =3D '\\'; \ - *wp++ =3D '0'; \ - *wp++ =3D '1'; \ - *wp++ =3D '1'; \ - } \ - else if (*rp =3D=3D '\n') \ - { \ - *wp++ =3D '\\'; \ - *wp++ =3D '0'; \ - *wp++ =3D '1'; \ - *wp++ =3D '2'; \ - } \ - else if (*rp =3D=3D '\\') \ - { \ - *wp++ =3D '\\'; \ - *wp++ =3D '\\'; \ - } \ - else \ - *wp++ =3D *rp; \ - while (*rp++ !=3D '\0'); \ - } \ - } while (0) - + while ((c =3D *str++) !=3D '\0') + { + if (strchr (encode_chars, c) =3D=3D NULL) + { + PUTC_OR_FAIL (c, stream); + } + else + { + PUTC_OR_FAIL ('\\', stream); + PUTC_OR_FAIL (((c & 0xc0) >> 6) + '0', stream); + PUTC_OR_FAIL (((c & 0x38) >> 3) + '0', stream); + PUTC_OR_FAIL (((c & 0x07) >> 0) + '0', stream); + } + } + PUTC_OR_FAIL (' ', stream); + return 0; +} =20 /* 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 =3D *mnt; + int ret =3D 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) !=3D 0); + return ret; + + flockfile (stream); + + ret =3D (write_string (stream, mnt->mnt_fsname) < 0 + || write_string (stream, mnt->mnt_dir) < 0 + || write_string (stream, mnt->mnt_type) < 0 + || write_string (stream, mnt->mnt_opts) < 0 + || fprintf (stream, "%d %d\n", mnt->mnt_freq, mnt->mnt_passno) < 0 + || fflush (stream) !=3D 0); + + funlockfile (stream); + + return ret; } weak_alias (__addmntent, addmntent) =20 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[] =3D +{ + {"/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 =3D 0; i < sizeof (tests) / sizeof (struct const_mntent); i= ++) + { + char buf[128]; + struct mntent *ret, curtest; + FILE *fp =3D fmemopen (buf, sizeof (buf), "w+"); + + if (fp =3D=3D NULL) + { + printf ("Failed to open file\n"); + return 1; + } + + curtest.mnt_fsname =3D strdupa (tests[i].mnt_fsname); + curtest.mnt_dir =3D strdupa (tests[i].mnt_dir); + curtest.mnt_type =3D strdupa (tests[i].mnt_type); + curtest.mnt_opts =3D strdupa (tests[i].mnt_opts); + curtest.mnt_freq =3D tests[i].mnt_freq; + curtest.mnt_passno =3D tests[i].mnt_passno; + + if (addmntent (fp, &curtest) !=3D 0) + { + support_record_failure (); + continue; + } + + TEST_COMPARE_STRING (buf, tests[i].expected); + + rewind (fp); + ret =3D getmntent (fp); + if (ret =3D=3D 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 --=20 2.29.2