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/