From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: zsh-workers-return-43796-ml=inbox.vuxu.org@zsh.org X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE autolearn=ham autolearn_force=no version=3.4.2 Received: from primenet.com.au (ns1.primenet.com.au [203.24.36.2]) by inbox.vuxu.org (OpenSMTPD) with ESMTP id 62f3a1df for ; Wed, 7 Nov 2018 14:35:31 +0000 (UTC) Received: (qmail 17538 invoked by alias); 7 Nov 2018 14:35:21 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: List-Unsubscribe: X-Seq: 43796 Received: (qmail 11205 invoked by uid 1010); 7 Nov 2018 14:35:21 -0000 X-Qmail-Scanner-Diagnostics: from mx1.redhat.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(209.132.183.28):SA:0(-6.9/5.0):. Processed in 2.23882 secs); 07 Nov 2018 14:35:21 -0000 X-Envelope-From: kdudka@redhat.com X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | From: Kamil Dudka To: Mikael Magnusson Cc: zsh-workers@zsh.org Subject: Re: [PATCH 4/5] Src/module: fix use-after-free in setmathfuncs() Date: Wed, 07 Nov 2018 15:35:52 +0100 Message-ID: <5400649.Fj7YaGC3tj@kdudka-nb> In-Reply-To: References: <20181107130456.18901-1-kdudka@redhat.com> <20181107130456.18901-4-kdudka@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.42]); Wed, 07 Nov 2018 14:35:15 +0000 (UTC) On Wednesday, November 7, 2018 3:21:31 PM CET Mikael Magnusson wrote: > On 11/7/18, Kamil Dudka wrote: > > Detected by Coverity Analysis: > > > > Error: USE_AFTER_FREE (CWE-825): > > zsh-5.5.1/Src/module.c:1390: freed_arg: "deletemathfunc" frees "f". > > zsh-5.5.1/Src/module.c:1352:6: freed_arg: "zfree" frees parameter "f". > > zsh-5.5.1/Src/mem.c:1888:5: freed_arg: "free" frees parameter "p". > > zsh-5.5.1/Src/module.c:1394: deref_after_free: Dereferencing freed pointer > > "f". > > 1392| ret = 1; > > 1393| } else { > > 1394|-> f->flags &= ~MFF_ADDED; > > 1395| } > > 1396| } > > --- > > > > Src/module.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/Src/module.c b/Src/module.c > > index 4ae78310f..33d75ebbd 100644 > > --- a/Src/module.c > > +++ b/Src/module.c > > @@ -1390,8 +1390,6 @@ setmathfuncs(char const *nam, MathFunc f, int size, > > int *e) > > > > if (deletemathfunc(f)) { > > > > zwarnnam(nam, "math function `%s' already deleted", f->name); > > ret = 1; > > > > - } else { > > - f->flags &= ~MFF_ADDED; > > > > } > > > > } > > f++; > > > > -- > > 2.17.2 > > In the other branch, if f was already deleted, how can we use f->name there? Do you mean deleted by the call of deletemathfunc(f) from setmathfuncs()? If deletemathfunc(f) returns -1, it did not delete anything. If deletemathfunc(f) returns 0, it either called free(f) or already executed: f->flags &= ~MFF_ADDED Executing it in setmathfuncs() again could hardly do anything useful. It would only cause use-after-free in case f->module was true. Kamil