From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/12810 Path: news.gmane.org!.POSTED!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] iconv: add check to avoid writing past end of buffer Date: Mon, 7 May 2018 15:28:21 -0400 Message-ID: <20180507192821.GL1392@brightrain.aerifal.cx> References: Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1525721195 24030 195.159.176.226 (7 May 2018 19:26:35 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Mon, 7 May 2018 19:26:35 +0000 (UTC) User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-12826-gllmg-musl=m.gmane.org@lists.openwall.com Mon May 07 21:26:31 2018 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1fFlmD-00060K-7l for gllmg-musl@m.gmane.org; Mon, 07 May 2018 21:26:25 +0200 Original-Received: (qmail 28494 invoked by uid 550); 7 May 2018 19:28:33 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 28473 invoked from network); 7 May 2018 19:28:33 -0000 Content-Disposition: inline In-Reply-To: Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:12810 Archived-At: On Wed, May 02, 2018 at 12:07:17PM -0500, Will Dietz wrote: > Attached. > > Example based on [1] that crashes without this fix can be found here: > > https://gist.github.com/7bc07da1dcd02e01c2fbb28cbaa81420 > > Input is from git's tests (2.17.0), and fixes tests when using > noxcuse-based iconv utility and musl's iconv implementation. > > Well, *almost*. At least no more crashing :). One final test > involving autosquash fails-- I believe due to a comparison breaking > due to unexpected shifts in ISO-2022-JP encoding (as described in [2]) > but I'm not sure of details just yet. Neat to get this far! > > ~Will > > [1] http://www.openwall.com/lists/musl/2017/05/03/1 > [2] http://www.openwall.com/lists/musl/2014/11/09/1 > From d4516bbca6b315927b82252baa24574ae12f0b06 Mon Sep 17 00:00:00 2001 > From: Will Dietz > Date: Tue, 1 May 2018 14:16:44 -0500 > Subject: [PATCH] iconv.c: add missing check against output buffer size > > --- > src/locale/iconv.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/locale/iconv.c b/src/locale/iconv.c > index d469856c..3c1f4dd2 100644 > --- a/src/locale/iconv.c > +++ b/src/locale/iconv.c > @@ -539,6 +539,7 @@ size_t iconv(iconv_t cd, char **restrict in, size_t *restrict inb, char **restri > if (*outb < 1) goto toobig; > if (c<256 && c==legacy_map(tomap, c)) { > revout: > + if (*outb < 1) goto toobig; > *(*out)++ = c; > *outb -= 1; > break; > -- > 2.17.0 > This also looks correct. I'm not too fond of the location of the check but I'm not sure there's anywhere I'd like much better. The other options look like at the beginning of each relevant case or before the whole switch. In some ways before the switch is nice (eliminates lots of duplicate checks) but it's redundant in the case of dest encodings that are always >1 byte. Rich