From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/6078 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] fix handling of zero length domain names in dn_expand Date: Thu, 4 Sep 2014 18:41:20 -0400 Message-ID: <20140904224120.GG23797@brightrain.aerifal.cx> References: <1407918101-3407-1-git-send-email-ncopa@alpinelinux.org> <20140904095039.496f5810@ncopa-desktop.alpinelinux.org> <20140904170726.GC10361@port70.net> <20140904181128.GD10361@port70.net> <20140904202053.GD23797@brightrain.aerifal.cx> <20140904221551.GH10361@port70.net> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1409870500 29753 80.91.229.3 (4 Sep 2014 22:41:40 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 4 Sep 2014 22:41:40 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-6091-gllmg-musl=m.gmane.org@lists.openwall.com Fri Sep 05 00:41:34 2014 Return-path: Envelope-to: gllmg-musl@plane.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1XPfif-0005aN-Pf for gllmg-musl@plane.gmane.org; Fri, 05 Sep 2014 00:41:33 +0200 Original-Received: (qmail 7409 invoked by uid 550); 4 Sep 2014 22:41:32 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 7395 invoked from network); 4 Sep 2014 22:41:32 -0000 Content-Disposition: inline In-Reply-To: <20140904221551.GH10361@port70.net> User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:6078 Archived-At: On Fri, Sep 05, 2014 at 12:15:52AM +0200, Szabolcs Nagy wrote: > * Rich Felker [2014-09-04 16:20:53 -0400]: > > > char *dend = dest + (space > 254 ? 254 : space); > > > - int len = -1, i, j; > > > - if (p==end || !*p) return -1; > > > + int len = -1, i, j, first = 1; > > > + if (p==end || dest==dend) return -1; > > > > Note that this does nothing to handle negative space, whereas ncopa's > > i assumed it to be ub Yes, technically it probably should be considered as such, but I was thinking of crazy things like large buffer lengths silently getting converted from size_t to int due to the poor choice of types for the signature. I definitely wouldn't want to treat negative values as large unsigned values -- this could overflow a small buffer, and real negatives could happen if "remaining buffer length" was computed poorly and not checked. One could argue that all of these considerations are beyond the scope of what the function needs to do, but checing is easy, and I'm so sick of bugs and potential vulns from this function already... > > such. Semantically it seems to be a zero-length component at the end > > (corresponding to "example.com.." rather than "example.com.", in the > > notation where final dots are not elided). Understanding whether it's > > legal or not probably requires some language-lawyering with RFC 1035, > > the rfc is not very clear but i think this case should work > > a name is a sequence of labels terminated by a 0 length label > > a compressed name is a leading sequence of labels terminated > by a pointer that can be expanded to a name > > if 'sequence of labels' can be empty and the conversion to > dotted string notation is done after concatenating the > leading and trailing sequence then there is no '..' issue OK, based on this explanation, I think both forms are valid. I always get stuck in the trap of reasoning in the mechanism of conversion rather than the "sequence of labels" abstraction. > > - char *dend = dest + (space > 254 ? 254 : space); > > + char *dend; > > int len = -1, i, j; > > - if (p==end || !*p) return -1; > > + if (p==end || size <= 0) return -1; > > + dend = dest + (space > 254 ? 254 : space); > > ok this part is better than mine Yes, whatever else we do, I think this is a nice addition. Rich