From patchwork Mon Mar 26 04:37:45 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Harald van Dijk X-Patchwork-Id: 10307117 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id E977960212 for ; Mon, 26 Mar 2018 04:36:38 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D627629435 for ; Mon, 26 Mar 2018 04:36:38 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C799629464; Mon, 26 Mar 2018 04:36:38 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI, T_TVD_MIME_EPI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 97BE529435 for ; Mon, 26 Mar 2018 04:36:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750808AbeCZEgh (ORCPT ); Mon, 26 Mar 2018 00:36:37 -0400 Received: from home.gigawatt.nl ([83.163.3.213]:47812 "EHLO home.gigawatt.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750761AbeCZEgg (ORCPT ); Mon, 26 Mar 2018 00:36:36 -0400 Received: from [IPv6:2001:980:4809:1:e045:1301:c405:78bf] (unknown [IPv6:2001:980:4809:1:e045:1301:c405:78bf]) by home.gigawatt.nl (Postfix) with ESMTPSA id 5975E540008F; Mon, 26 Mar 2018 04:36:34 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 home.gigawatt.nl 5975E540008F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gigawatt.nl; s=default; t=1522038994; bh=tsyGlPCZiRZ5OWUw7u+VAnmTSrWIVgwsU8Uq+TgitRM=; l=3442; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=EfjHg6GWgaW9EpExHxm0U/ujQDzUY8wHmV47ULnXk9Srb5YhtL9xUm9hVH319+hFz olxJ2jeO99HHI8afv19MEgqagwv0Puhftm7EL5peTl8VkziGi9cVpycQnexi/P78Nd u7eUBBLxID6RDJaErfCa4eOdVM4dWso/pRs4/7BU= Subject: Re: expand: Fix buffer overflow in expandmeta To: Herbert Xu Cc: DASH Mailing List References: <20180325083800.GA5496@gondor.apana.org.au> <9f33d52a-fefa-edf3-54c0-ab2fe6dcd47b@gigawatt.nl> <20180326025410.GA6913@gondor.apana.org.au> From: Harald van Dijk Message-ID: <98dc5115-ce2e-ef9a-27dc-d865165ae2cd@gigawatt.nl> Date: Mon, 26 Mar 2018 06:37:45 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101 Thunderbird/59.0 MIME-Version: 1.0 In-Reply-To: <20180326025410.GA6913@gondor.apana.org.au> Content-Language: en-US Sender: dash-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dash@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 3/26/18 4:54 AM, Herbert Xu wrote: > On Mon, Mar 26, 2018 at 03:03:38AM +0200, Harald van Dijk wrote: >> >> This was already the case before your patch, but on systems that flat out >> reject paths longer than PATH_MAX (that includes GNU/Linux), it seems weird >> that expansion can produce paths which then don't work. > > PATH_MAX only applies in certain cases. Besides, you can always > cd into the directories one level at a time to circumvent it. In other words, when you change the path to a different one than what globbing produced. That was kind of my point. :) > Besides, even if we did impose a limit here > we'd still have to check that limit at every recursion level which > means that the code won't be that much simpler. Proof of concept attached. The resulting code is about 100 bytes smaller at -Os than the dynamic reallocation approach. It's possible to further reduce that with a statically allocated buffer, but of course that means a constant memory cost at run time. Cheers, Harald van Dijk --- a/src/expand.c +++ b/src/expand.c @@ -122,7 +122,7 @@ STATIC void expandmeta(struct strlist *, int); #ifdef HAVE_GLOB STATIC void addglob(const glob_t *); #else -STATIC void expmeta(char *, char *); +STATIC void expmeta(char *); STATIC struct strlist *expsort(struct strlist *); STATIC struct strlist *msort(struct strlist *, int); #endif @@ -1237,9 +1237,6 @@ addglob(pglob) #else /* HAVE_GLOB */ -STATIC char *expdir; - - STATIC void expandmeta(struct strlist *str, int flag) { @@ -1261,13 +1258,7 @@ expandmeta(struct strlist *str, int flag) INTOFF; p = preglob(str->text, RMESCAPE_ALLOC | RMESCAPE_HEAP); - { - int i = strlen(str->text); - expdir = ckmalloc(i < 2048 ? 2048 : i); /* XXX */ - } - - expmeta(expdir, p); - ckfree(expdir); + expmeta(p); if (p != str->text) ckfree(p); INTON; @@ -1296,7 +1287,7 @@ nometa: */ STATIC void -expmeta(char *enddir, char *name) +expmeta1(char *expdir, char *enddir, char *name) { char *p; const char *cp; @@ -1344,6 +1335,8 @@ expmeta(char *enddir, char *name) metaflag++; p = name; do { + if (enddir == expdir + PATH_MAX) + return; if (*p == '\\') p++; *enddir++ = *p; @@ -1390,22 +1383,34 @@ expmeta(char *enddir, char *name) if (dp->d_name[0] == '.' && ! matchdot) continue; if (pmatch(start, dp->d_name)) { + p = enddir; + cp = dp->d_name; + for (;;) { + if (p == expdir + PATH_MAX) + goto toolong; + if ((*p++ = *cp++) == '\0') + break; + } if (atend) { - scopy(dp->d_name, enddir); addfname(expdir); } else { - for (p = enddir, cp = dp->d_name; - (*p++ = *cp++) != '\0';) - continue; p[-1] = '/'; - expmeta(p, endname); + expmeta1(expdir, p, endname); } } +toolong: ; } closedir(dirp); if (! atend) endname[-esc - 1] = esc ? '\\' : '/'; } + +STATIC void +expmeta(char *name) +{ + char expdir[PATH_MAX]; + expmeta1(expdir, expdir, name); +} #endif /* HAVE_GLOB */