bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

tar + cpio - covscan issues


From: Ondrej Dubaj
Subject: tar + cpio - covscan issues
Date: Thu, 8 Apr 2021 13:37:26 +0200

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 = "">+        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);

reply via email to

[Prev in Thread] Current Thread [Next in Thread]