Re: [PATCH] fix handling of zero length domain names in dn_expand

[Rich Felker <dalias@libc.org>]

On Fri, Sep 05, 2014 at 12:15:52AM +0200, Szabolcs Nagy wrote:
> * Rich Felker <dalias@libc.org> [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