From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2840 invoked by alias); 27 Mar 2018 22:04:20 -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: 42546 Received: (qmail 5102 invoked by uid 1010); 27 Mar 2018 22:04:20 -0000 X-Qmail-Scanner-Diagnostics: from mx3-rdu2.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(66.187.233.73):SA:0(-1.9/5.0):. Processed in 15.174342 secs); 27 Mar 2018 22:04:20 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,T_RP_MATCHES_RCVD, T_SPF_HELO_PERMERROR,T_SPF_TEMPERROR autolearn=ham autolearn_force=no version=3.4.1 X-Envelope-From: kdudka@redhat.com X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | From: Kamil Dudka To: Oliver Kiddle Cc: zsh-workers@zsh.org Subject: Re: PATCH: spelling correction buffer overflow Date: Tue, 27 Mar 2018 23:55:50 +0200 Message-ID: <52497193.Jvz9bWjiNL@kdudka-nb> In-Reply-To: <3579.1522103995@thecus> References: <3579.1522103995@thecus> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Tue, 27 Mar 2018 21:55:27 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Tue, 27 Mar 2018 21:55:27 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kdudka@redhat.com' RCPT:'' On Tuesday, March 27, 2018 12:39:55 AM CEST Oliver Kiddle wrote: > In utils.c:spname(), there's no checking on newname[] for overflows in > two out the of three places where it is appended to. Thanks! After applying this patch, zsh does not crash any more while trying to correct an overly long command. Kamil > returning NULL appears to abort the command-line rather than aborting > only correction which is perhaps not ideal but the aim here is not to > corrupt other bits of memory. > > This also tweaks the struncpy() function I updated in the earlier patch. > > Oliver > > diff --git a/Src/utils.c b/Src/utils.c > index 998b16220..9ea34ab54 100644 > --- a/Src/utils.c > +++ b/Src/utils.c > @@ -2283,7 +2283,8 @@ struncpy(char **s, char *t, int n) > { > char *u = *s; > > - while (n-- && (*u++ = *t++)); > + while (n-- && (*u = *t++)) > + u++; > *s = u; > if (n > 0) /* just one null-byte will do, unlike strncpy(3) */ > *u = '\0'; > @@ -4420,17 +4421,20 @@ spname(char *oldname) > * odd to the human reader, and we may make use of the total * > * distance for all corrections at some point in the future. */ > if (bestdist < maxthresh) { > - strcpy(new, spnameguess); > - strcat(new, old); > - return newname; > + struncpy(&new, spnameguess, sizeof(newname) - (new - newname)); > + struncpy(&new, old, sizeof(newname) - (new - newname)); > + return (new - newname) >= (sizeof(newname)-1) ? NULL : newname; > } else > return NULL; > } else { > maxthresh = bestdist + thresh; > bestdist += thisdist; > } > - for (p = spnamebest; (*new = *p++);) > + for (p = spnamebest; (*new = *p++);) { > + if ((new - newname) >= (sizeof(newname)-1)) > + return NULL; > new++; > + } > } > }