Hello,
proposing patch for some of the issues found by coverity scan in tar-1.34
Patch:
diff --git a/gnu/malloc/scratch_buffer_dupfree.c
b/gnu/malloc/scratch_buffer_dupfree.c
index 775bff5..3b246f2 100644
--- a/gnu/malloc/scratch_buffer_dupfree.c
+++ b/gnu/malloc/scratch_buffer_dupfree.c
@@ -35,7 +35,13 @@ __libc_scratch_buffer_dupfree (struct scratch_buffer
*buffer, size_t size)
else
{
void *copy = realloc (data, size);
- return copy != NULL ? copy : data;
+ if (copy != NULL)
+ {
+ data = NULL;
+ return copy;
+ }
+ else
+ return data;
}
}
libc_hidden_def (__libc_scratch_buffer_dupfree)
diff --git a/lib/wordsplit.c b/lib/wordsplit.c
index 661a4f8..6ccaa2a 100644
--- a/lib/wordsplit.c
+++ b/lib/wordsplit.c
@@ -615,7 +615,6 @@ coalesce_segment (struct wordsplit *wsp, struct
wordsplit_node *node)
node->flags |= p->flags & _WSNF_QUOTE;
wsnode_remove (wsp, p);
stop = p == end;
- wsnode_free (p);
}
p = next;
}
In addition, there are some issues which are not resolved by this patch.
There is a compiler warning about issues in utimens.c, which I find as
false positives. Another false positive is memory leak in malloca.c. Issue
presented in stdopen.c might be actually a problem. Can you please
investigate it and give feedback ?
Covscan results:
Error: CPPCHECK_WARNING (CWE-401):
tar-1.34/gnu/malloc/scratch_buffer_dupfree.c:38: error[memleak]:
Memory leak: copy
# 36| {
# 37| void *copy = realloc (data, size);
# 38|-> return copy != NULL ? copy : data;
# 39| }
# 40| }
Error: CPPCHECK_WARNING (CWE-401):
tar-1.34/gnu/malloca.c:67: error[memleak]: Memory leak: mem
# 65| ((small_t *) p)[-1] = p - mem;
# 66| /* p sa_alignment_max mod 2*sa_alignment_max. */
# 67|-> return p;
# 68| }
# 69| }
Error: RESOURCE_LEAK (CWE-772):
tar-1.34/gnu/stdopen.c:51: open_fn: Returning handle opened by "open".
[Note: The source code implementation of the function has been
overridden by a user model.]
tar-1.34/gnu/stdopen.c:51: var_assign: Assigning: "full_fd" = handle
returned from "open("/dev/full", mode)".
tar-1.34/gnu/stdopen.c:52: var_assign: Assigning: "new_fd" = "full_fd".
tar-1.34/gnu/stdopen.c:62: leaked_handle: Handle variable "new_fd"
going out of scope leaks the handle.
tar-1.34/gnu/stdopen.c:62: leaked_handle: Handle variable "full_fd"
going out of scope leaks the handle.
# 60| return 0;
# 61| }
# 62|-> }
# 63| }
# 64|
Error: RESOURCE_LEAK (CWE-772):
tar-1.34/gnu/stdopen.c:52: open_fn: Returning handle opened by "open".
[Note: The source code implementation of the function has been
overridden by a user model.]
tar-1.34/gnu/stdopen.c:52: var_assign: Assigning: "new_fd" = handle
returned from "open("/dev/null", mode)".
tar-1.34/gnu/stdopen.c:62: leaked_handle: Handle variable "new_fd"
going out of scope leaks the handle.
# 60| return 0;
# 61| }
# 62|-> }
# 63| }
# 64|
Error: COMPILER_WARNING (CWE-758):
tar-1.34/gnu/utimens.c: scope_hint: In function 'fdutimens'
tar-1.34/gnu/utimens.c:399:17: warning[-Wstringop-overflow=]:
'update_timespec' accessing 16 bytes in a region of size 8
# 399 | if (ts && update_timespec (&st, &ts))
# | ^~~~~~~~~~~~~~~~~~~~~~~~~~
tar-1.34/gnu/utimens.c:399:17: note: referencing argument 2 of type
'struct timespec * *'
tar-1.34/gnu/utimens.c:136:1: note: in a call to function 'update_timespec'
# 136 | update_timespec (struct stat const *statbuf, struct timespec *ts[2])
# | ^~~~~~~~~~~~~~~
# 397| && (fd < 0 ? stat (file, &st) : fstat (fd, &st)))
# 398| return -1;
# 399|-> if (ts && update_timespec (&st, &ts))
# 400| return 0;
# 401| }
Error: COMPILER_WARNING (CWE-758):
tar-1.34/gnu/utimens.c: scope_hint: In function 'lutimens'
tar-1.34/gnu/utimens.c:612:17: warning[-Wstringop-overflow=]:
'update_timespec' accessing 16 bytes in a region of size 8
# 612 | if (ts && update_timespec (&st, &ts))
# | ^~~~~~~~~~~~~~~~~~~~~~~~~~
tar-1.34/gnu/utimens.c:612:17: note: referencing argument 2 of type
'struct timespec * *'
tar-1.34/gnu/utimens.c:136:1: note: in a call to function 'update_timespec'
# 136 | update_timespec (struct stat const *statbuf, struct timespec *ts[2])
# | ^~~~~~~~~~~~~~~
# 610| if (adjustment_needed != 3 && lstat (file, &st))
# 611| return -1;
# 612|-> if (ts && update_timespec (&st, &ts))
# 613| return 0;
# 614| }
Error: USE_AFTER_FREE (CWE-416):
tar-1.34/lib/wordsplit.c:683: freed_arg: "coalesce_segment" frees "p->next".
tar-1.34/lib/wordsplit.c:680: use_after_free: Using freed pointer "p->next".
# 678| struct wordsplit_node *p;
# 679|
# 680|-> for (p = wsp->ws_head; p; p = p->next)
# 681| {
# 682| if (p->flags & _WSNF_JOIN)
As well, proposing patch for 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
@@ -173,8 +174,9 @@ write_out_tar_header (struct cpio_file_stat *file_hdr,
int out_des)
{
/* process_copy_out makes sure that c_tar_linkname is shorter
than TARLINKNAMESIZE. */
+ memset(tar_hdr->linkname, '\0', TARLINKNAMESIZE);
strncpy (tar_hdr->linkname, file_hdr->c_tar_linkname,
- TARLINKNAMESIZE);
+ TARLINKNAMESIZE-1);
tar_hdr->typeflag = LNKTYPE;
to_ascii (tar_hdr->size, 0, 12, LG_8, true);
}
@@ -200,8 +202,9 @@ write_out_tar_header (struct cpio_file_stat *file_hdr,
int out_des)
tar_hdr->typeflag = SYMTYPE;
/* process_copy_out makes sure that c_tar_linkname is shorter
than TARLINKNAMESIZE. */
+ memset(tar_hdr->linkname, '\0', TARLINKNAMESIZE);
strncpy (tar_hdr->linkname, file_hdr->c_tar_linkname,
- TARLINKNAMESIZE);
+ TARLINKNAMESIZE-1);
to_ascii (tar_hdr->size, 0, 12, LG_8, true);
break;
#endif /* CP_IFLNK */
@@ -211,6 +214,7 @@ write_out_tar_header (struct cpio_file_stat *file_hdr,
int out_des)
{
char *name;
+ memset(tar_hdr->version, '\0', TVERSLEN+1);
strncpy (tar_hdr->magic, TMAGIC, TMAGLEN);
strncpy (tar_hdr->version, TVERSION, TVERSLEN);
In addition, there are some issues which are not resolved by this patch.
There is a compiler warning about issues in utimens.c (same as in tar),
which I find as false positives. Can you please investigate it and give
feedback ?
Thank you.
Ondrej
Covscan results:
Error: COMPILER_WARNING (CWE-758):
cpio-2.13/gnu/utimens.c: scope_hint: In function 'fdutimens'
cpio-2.13/gnu/utimens.c:296:17: warning[-Wstringop-overflow=]:
'update_timespec' accessing 16 bytes in a region of size 8
# 296 | if (ts && update_timespec (&st, &ts))
# | ^~~~~~~~~~~~~~~~~~~~~~~~~~
cpio-2.13/gnu/utimens.c:296:17: note: referencing argument 2 of type
'struct timespec * *'
cpio-2.13/gnu/utimens.c:131:1: note: in a call to function 'update_timespec'
# 131 | update_timespec (struct stat const *statbuf, struct timespec *ts[2])
# | ^~~~~~~~~~~~~~~
# 294| && (fd < 0 ? stat (file, &st) : fstat (fd, &st)))
# 295| return -1;
# 296|-> if (ts && update_timespec (&st, &ts))
# 297| return 0;
# 298| }
Error: COMPILER_WARNING (CWE-758):
cpio-2.13/gnu/utimens.c: scope_hint: In function 'lutimens'
cpio-2.13/gnu/utimens.c:507:17: warning[-Wstringop-overflow=]:
'update_timespec' accessing 16 bytes in a region of size 8
# 507 | if (ts && update_timespec (&st, &ts))
# | ^~~~~~~~~~~~~~~~~~~~~~~~~~
cpio-2.13/gnu/utimens.c:507:17: note: referencing argument 2 of type
'struct timespec * *'
cpio-2.13/gnu/utimens.c:131:1: note: in a call to function 'update_timespec'
# 131 | update_timespec (struct stat const *statbuf, struct timespec *ts[2])
# | ^~~~~~~~~~~~~~~
# 505| if (adjustment_needed != 3 && lstat (file, &st))
# 506| return -1;
# 507|-> if (ts && update_timespec (&st, &ts))
# 508| return 0;
# 509| }
Error: COMPILER_WARNING (CWE-758):
cpio-2.13/src/tar.c: scope_hint: In function 'write_out_tar_header'
cpio-2.13/src/tar.c:149:7: warning[-Wstringop-overflow=]: 'strncpy'
specified bound depends on the length of the source argument
cpio-2.13/src/tar.c:146:14: note: length computed here
# 147| if (name_len <= TARNAMESIZE)
# 148| {
# 149|-> strncpy (tar_hdr->name, file_hdr->c_name, name_len);
# 150| }
# 151| else
Error: BUFFER_SIZE (CWE-170):
cpio-2.13/src/tar.c:176: buffer_size_warning: Calling "strncpy" with a
maximum size argument of 100 bytes on destination array
"tar_hdr->linkname" of size 100 bytes might leave the destination
string unterminated.
# 174| /* process_copy_out makes sure that c_tar_linkname is shorter
# 175| than TARLINKNAMESIZE. */
# 176|-> strncpy (tar_hdr->linkname, file_hdr->c_tar_linkname,
# 177| TARLINKNAMESIZE);
# 178| tar_hdr->typeflag = LNKTYPE;
Error: BUFFER_SIZE (CWE-170):
cpio-2.13/src/tar.c:203: buffer_size_warning: Calling "strncpy" with a
maximum size argument of 100 bytes on destination array
"tar_hdr->linkname" of size 100 bytes might leave the destination
string unterminated.
# 201| /* process_copy_out makes sure that c_tar_linkname is shorter
# 202| than TARLINKNAMESIZE. */
# 203|-> strncpy (tar_hdr->linkname, file_hdr->c_tar_linkname,
# 204| TARLINKNAMESIZE);
# 205| to_ascii (tar_hdr->size, 0, 12, LG_8, true);
Error: BUFFER_SIZE (CWE-120):
cpio-2.13/src/tar.c:215: buffer_size: Calling "strncpy" with a source
string whose length (2 chars) is greater than or equal to the size
argument (2) will fail to null-terminate "tar_hdr->version".
# 213|
# 214| strncpy (tar_hdr->magic, TMAGIC, TMAGLEN);
# 215|-> strncpy (tar_hdr->version, TVERSION, TVERSLEN);
# 216|
# 217| name = getuser (file_hdr->c_uid);
