Re: [Bug-cpio] [PATCH] fix 1-byte out-of-bounds write

[Pavel Raiskup]

On Thursday, April 6, 2017 3:01:46 PM CEST Kristýna Streitová wrote:
> Thank you for it. I have just small comments on your patch:
> 
> 1) It seems that the first hunk slightly changes the logic.
> Before your patch, the 'is_tar_filename_too_long()' function was called
> for tar and ustar archives only but now it's called for all archive types.

good catch.  It looked like a typo to me before (that non-tar archives were not
checked), but looking more precisely -- I would say it is useless check.
Previously there was comment:

  /* Debian hack: file_hrd.c_name is sometimes set to
     point to static memory by code in tar.c.  This
     causes a segfault.  This has been fixed and an
     additional check to ensure that the file name
     is not too long has been added.  (Reported by
     Horst Knobloch.)  This bug has been reported to
     "address@hidden". (99/1/6) -BEM */

I'm not sure what the "additional check" note talks about, and I'm not
able to find the original report from 1999 ...

Because --rename and --rename-batch-file is valid for copy-in mode, I
think removing the check is completely safe.

> As this function checks whether the filename is too long to fit in a tar
> header, it should be called for tar/ustar archives only, I believe.
> 
> 2) It's just a small thing but it seems that you forgot to use your new
> 'cpio_set_c_name()' function for 'file_hdr.c_name = CPIO_TRAILER_NAME'
> in the copyout.c file.

That's almost before exit(), but let's be consistent ... thanks for the
review.

Pavel

> Best regards,
> Kristyna Streitova
>

0001-CVE-2016-2037-1-byte-out-of-bounds-write.patch
Description: Text Data