From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS22989 209.51.188.0/24 X-Spam-Status: No, score=-3.7 required=3.0 tests=AWL,BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 0161E1F466 for ; Fri, 17 Jan 2020 21:00:50 +0000 (UTC) Received: from localhost ([::1]:34546 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1isYjZ-0001Cn-KF for normalperson@yhbt.net; Fri, 17 Jan 2020 16:00:49 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:49798) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1isYjT-0001CN-Rl for bug-gnulib@gnu.org; Fri, 17 Jan 2020 16:00:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1isYjS-00037x-En for bug-gnulib@gnu.org; Fri, 17 Jan 2020 16:00:43 -0500 Received: from mo6-p00-ob.smtp.rzone.de ([2a01:238:20a:202:5300::1]:23353) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1isYjR-00031E-ND for bug-gnulib@gnu.org; Fri, 17 Jan 2020 16:00:42 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1579294838; s=strato-dkim-0002; d=clisp.org; h=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From: X-RZG-CLASS-ID:X-RZG-AUTH:From:Subject:Sender; bh=2G4M3LcEViobs2A/ooqxOBCk4hO6RNqFPhUIHPLwKJM=; b=LlMigEEeYX1XmcAsKDCChOqJpQZkEBQgM6r2jO6tjHxgnzE6zYjiQ9vUds53TopXBT lZ7St4pIIahr4GxzIp7kYWhVsTNFWyHim5Ufwwg6QQoOA0j20p3PKx7O/EcHsnOn5kk3 pvcZXCu2gmUEKoDW2ilYv1b0q7/ed2al+eJP7v57JlNGTxjjDZOcnHHN/bPar/DWGVZT Mdmnauu0w88yfgE8eD7xLE0NGmakrTfyCbODTSVkj7ZzcASRtYzfTpD7+rZ1ICqfNVCh 37T05iGepEpHGy8x1nnNKD/wj+gmBgW/K8y/RkO2dpDidpdiekP3pQrKxf2kCPNEYm08 s6Og== X-RZG-AUTH: ":Ln4Re0+Ic/6oZXR1YgKryK8brlshOcZlIWs+iCP5vnk6shH+AHjwLuWOH6fzxfs=" X-RZG-CLASS-ID: mo00 Received: from bruno.haible.de by smtp.strato.de (RZmta 46.1.4 DYNA|AUTH) with ESMTPSA id z0b9d9w0HL0ahNT (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (curve X9_62_prime256v1 with 256 ECDH bits, eq. 3072 bits RSA)) (Client did not present a certificate); Fri, 17 Jan 2020 22:00:36 +0100 (CET) From: Bruno Haible To: Paul Eggert Subject: Re: heap-use-after-free in rpl_glob Date: Fri, 17 Jan 2020 22:00:35 +0100 Message-ID: <2031383.ETbFSIWNRB@omega> User-Agent: KMail/5.1.3 (Linux/4.4.0-171-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: <3c926e49-6d61-21d5-7688-d830b029341c@cs.ucla.edu> References: <6008915.UEnMCXWWPA@omega> <3c926e49-6d61-21d5-7688-d830b029341c@cs.ucla.edu> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="nextPart3364248.Ed5lFiqo68" Content-Transfer-Encoding: 7Bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a01:238:20a:202:5300::1 X-BeenThere: bug-gnulib@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Gnulib discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Tim =?ISO-8859-1?Q?R=FChsen?= , bug-gnulib@gnu.org Errors-To: bug-gnulib-bounces+normalperson=yhbt.net@gnu.org Sender: "bug-gnulib" This is a multi-part message in MIME format. --nextPart3364248.Ed5lFiqo68 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Hi Paul, > It seems that your patch is incomplete, as there's a memory leak in the > unlikely case that the malloc call fails. Oops, indeed. Thanks for the review! > Something like the attached patch instead, perhaps? Hmm, this patch is freeing the same object through the variable 'dirname' on one branch and through the variable 'previous_dirname' on the other branch. A bit hard to understand, and thus likely to introduce more bugs in the future, I would say. I prefer the attached one. Pushed. Bruno --nextPart3364248.Ed5lFiqo68 Content-Disposition: attachment; filename="0001-glob-Fix-use-after-free-bug.patch" Content-Transfer-Encoding: quoted-printable Content-Type: text/x-patch; charset="UTF-8"; name="0001-glob-Fix-use-after-free-bug.patch" =46rom 717766da8926e36cf86015c4a49554baa854e8e6 Mon Sep 17 00:00:00 2001 =46rom: Bruno Haible Date: Fri, 17 Jan 2020 21:56:01 +0100 Subject: [PATCH] glob: Fix use-after-free bug. MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit Reported by Tim R=C3=BChsen in . * lib/glob.c (__glob): Delay freeing dirname until after the use of end_name. =2D-- ChangeLog | 9 +++++++++ lib/glob.c | 8 ++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 643dba3..4f4718a 100644 =2D-- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2020-01-17 Bruno Haible + Paul Eggert + + glob: Fix use-after-free bug. + Reported by Tim R=C3=BChsen in + . + * lib/glob.c (__glob): Delay freeing dirname until after the use of + end_name. + 2020-01-16 Siddhesh Poyarekar =20 vcs-to-changelog: Fix parsing of fndecl without args. diff --git a/lib/glob.c b/lib/glob.c index a67cbb6..add5d93 100644 =2D-- a/lib/glob.c +++ b/lib/glob.c @@ -843,10 +843,11 @@ __glob (const char *pattern, int flags, int (*errfunc= ) (const char *, int), { size_t home_len =3D strlen (p->pw_dir); size_t rest_len =3D end_name =3D=3D NULL ? 0 : strlen (end= _name); + /* dirname contains end_name; we can't free it now. */ + char *prev_dirname =3D + (__glibc_unlikely (malloc_dirname) ? dirname : NULL); char *d; =20 =2D if (__glibc_unlikely (malloc_dirname)) =2D free (dirname); malloc_dirname =3D 0; =20 if (glob_use_alloca (alloca_used, home_len + rest_len + 1)) @@ -857,6 +858,7 @@ __glob (const char *pattern, int flags, int (*errfunc) = (const char *, int), dirname =3D malloc (home_len + rest_len + 1); if (dirname =3D=3D NULL) { + free (prev_dirname); scratch_buffer_free (&pwtmpbuf); retval =3D GLOB_NOSPACE; goto out; @@ -868,6 +870,8 @@ __glob (const char *pattern, int flags, int (*errfunc) = (const char *, int), d =3D mempcpy (d, end_name, rest_len); *d =3D '\0'; =20 + free (prev_dirname); + dirlen =3D home_len + rest_len; dirname_modified =3D 1; } =2D-=20 2.7.4 --nextPart3364248.Ed5lFiqo68--