Subject: Re: gzip buffer overflow found
To: enami tsugutomo <enami@sm.sony.co.jp>
From: Simon Burge <simonb@wasabisystems.com>
List: current-users
Date: 01/22/2001 21:57:35
enami tsugutomo wrote:

> Simon Burge <simonb@wasabisystems.com> writes:
> 
> > -            strcpy(z_suffix, optarg);
> > +	    if (z_len > sizeof(z_suffix)-1) {
> > +		fprintf(stderr, "%s: -S suffix too long\n", progname);
> > +		usage();
> > +		do_exit(ERROR);
> > +	    }
> > +            strlcpy(z_suffix, optarg, sizeof(z_suffix));
> 
> Probably it is better to use the same way to detect overflow written
> in man page, isn't it?

Sorry, I can't see a reference in the man page on this.  Can you please
quote the section?

> > -        if (*dot == '\0') strcpy(dot, ".");
> > +        if (*dot == '\0')
> > +	    strlcpy(dot, ".", sizeof(dot));
> 
> You have to check the type of ``dot'' more carefully.

Yes, this is wrong...

> > @@ -1473,7 +1480,7 @@ local void shorten_name(name)
> >      len = strlen(name);
> >      if (decompress) {
> >  	if (len <= 1) error("name too short");
> > -	name[len-1] = '\0';
> > +	name[0] = '\0';
> >  	return;
> >      }
> >      p = get_suffix(name);
> 
> Shotening directly to null string is too aggressive :-).  I guess
> original intention is shotening one by one.

Yes, I was fooled by the lack of KNF here and tried to simplify it too
much.  The benefits of code review :)

Simon.
--
Simon Burge                            <simonb@wasabisystems.com>
NetBSD CDs, Support and Service:    http://www.wasabisystems.com/