On Sat, Jan 20, 2001 at 07:31:23AM +0100, Reinoud Zandijk wrote:
> Hi Simon,
> 
> euhm... maybe i am a bit late, but i would like to make some small
> comments on the patch you provided in your message :
> 
> On Fri, 19 Jan 2001, Simon Burge wrote:
> 
> <SNIP>
> 
> > @@ -635,8 +640,8 @@ local void treat_stdin()
> >      if (!test && !list && (!decompress || !ascii)) {
> >  	SET_BINARY_MODE(fileno(stdout));
> >      }
> > -    strcpy(ifname, "stdin");
> > -    strcpy(ofname, "stdout");
> > +    strlcpy(ifname, "stdin", sizeof(ifname));
> > +    strlcpy(ofname, "stdout", sizeof(ofname));
> >  
> >      /* Get the time stamp on the input file. */
> >      time_stamp = 0; /* time unknown by default */
> > @@ -751,7 +756,7 @@ local void treat_file(iname)
> >       * without a valid gzip suffix (check done in make_ofname).
> >       */
> >      if (to_stdout && !list && !test) {
> > -	strcpy(ofname, "stdout");
> > +	strlcpy(ofname, "stdout", sizeof(ofname));
> 
> I know this might be a bit of a strange remark... but why use
> `strlcpy(ifname, "stdin", sizeof(ifname));' .... ifname can hold at least
> the name `stdin' ... and copying like that might even copy some code/other
> data after the string constant making it even an exploit :(

  strlcpy() fixes that annoyance of strncpy() -- it only copies as
much as needed, up to a limit (with a guarantee of NUL termination to
boot, unlike strncpy()).  strncpy will continue to write 0's after the
end of the string, up to the limit, so strncpy(a,"b",1048576) will
write almost 1MB of zeroes.

  I just tested this out, and yes, strncpy goes forever adding 0's,
while strlcpy does the minimum memory traffic needed.

  So, my vote would be, keep it the safe strlcpy() way -- that way, if
"stdin" is later replaced by argv[i], we don't have to worry.  :)

  Brian Grayson