Re: cpio - covscan issues

[Martin Simmons]

>>>>> On Thu, 08 Apr 2021 13:37:13 +0200, Kamil Dudka said:
> 
> On Thursday, April 8, 2021 12:33:11 PM CEST Ondrej Dubaj wrote:
> > On Thu, Apr 8, 2021 at 12:02 PM Kamil Dudka <kdudka@redhat.com> wrote:
> > > On Thursday, April 8, 2021 9:47:05 AM CEST Ondrej Dubaj wrote:
> > > > Hello,
> > > > 
> > > > proposing patch for some of the issues found by coverity scan in
> > > 
> > > cpio-2.13
> > > 
> > > > Patch:
> > > > 
> > > > diff --git a/src/tar.c b/src/tar.c
> > > > index 99ef8a2..a5873e7 100644
> > > > --- a/src/tar.c
> > > > +++ b/src/tar.c
> > > > @@ -146,6 +146,7 @@ write_out_tar_header (struct cpio_file_stat
> > > 
> > > *file_hdr,
> > > 
> > > > int out_des)
> > > > 
> > > >    name_len = strlen (file_hdr->c_name);
> > > >    if (name_len <= TARNAMESIZE)
> > > >    
> > > >      {
> > > > 
> > > > +      memset(tar_hdr->name, '\0', name_len+1);
> > > > 
> > > >        strncpy (tar_hdr->name, file_hdr->c_name, name_len);
> > > >      
> > > >      }
> > > >    
> > > >    else
> > > 
> > > This is obviously incorrect because it would write past the tar_hdr->name
> > > array in case (name_len == TARNAMESIZE).
> > 
> > Actually you are right, the best option might be:
> > 
> > memset(tar_hdr->name, '\0', TARNAMESIZE);
> 
> This would not ensure NUL-termination either because the subsequent call to 
> strncpy() might overwrite all the zeros with non-zeros in case (name_len == 
> TARNAMESIZE).  I believe this would work better:
> 
>     strncpy (tar_hdr->name, file_hdr->c_name, name_len);
>     tar_hdr->name[TARNAMESIZE - 1] = '\0';

Does it have to be NUL-terminated in the tar header?  GNU tar doesn't
NUL-terminate filenames with length 100 in the v7 format:

$ touch 
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678x
$ tar -c --format=v7 -f - 
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678x
 | od -c | head -7
0000000   0   1   2   3   4   5   6   7   8   9   0   1   2   3   4   5
0000020   6   7   8   9   0   1   2   3   4   5   6   7   8   9   0   1
0000040   2   3   4   5   6   7   8   9   0   1   2   3   4   5   6   7
0000060   8   9   0   1   2   3   4   5   6   7   8   9   0   1   2   3
0000100   4   5   6   7   8   9   0   1   2   3   4   5   6   7   8   9
0000120   0   1   2   3   4   5   6   7   8   9   0   1   2   3   4   5
0000140   6   7   8   x   0   0   0   0   6   6   4  \0   0   0   0   1

The x is the last character of the name in the header, followed by the
mode 0000664 in octal.

The code should probably be using memcpy instead of strncpy because the
header has already been filled with NULs.

__Martin