I went ahead and tested a patch for this issue. This is my first contribution, so I hope it is formatted acceptably.

When looking at the extractor source, I wasn't sure about this section which switches a target directory to a "safe" permission mode then changes it back afterward. I'm not sure this logic follows the documented behavior of `--no-overwrite-dir` since it still updates the "Change"/"Modify" metadata fields. It seems to me if a target directory is not traversable, extraction with no-overwrite-dir should fail where it currently does not. Nonetheless, I'm sure there's a reason I'm simply not aware of, so this patch does preserve the existing behavior.

Thus my addition checks the euid to see if the current user is able to perform this permissions swap; and if the change would fail, do nothing instead. This is preferred for cases where users can have absolute paths to user-specific files/folders. For example, extracting a tar file containing './home/nate/myfile', using the user 'nate' fails in the current tar version, but passes with this patch.

Nate Simon


On 11/11/24 08:43, Nate Simon wrote:
Tar appears to be not preserving all metadata when --no-overwrite-dir is used for file extraction. I started seeing this regression when we updated from Ubuntu 20 to Ubuntu 22: tar 1.30 -> 1.34.

This is the example that made me aware of the issue. These commands pass with tar 1.30.

$ mkdir usr
$ tar -cf test.tar usr
$ tar --no-overwrite-dir -xf test.tar -C /
tar: usr: Cannot change mode to rwxr-xr-x: Operation not permitted
tar: Exiting with failure status due to previous errors


And here is a similar example with folder metadata. I observed that even with '--no-overwrite-dir' passed to tar, the folder's "modify" metadata is rolled back to match the folder contained in the tarball. But based on the documentation, I expected neither Modify nor Change fields to update.

$ mkdir test
$ tar -cf test2.tar test
$ rmdir test
$ mkdir test
$ stat test
   File: test
   Size: 4096          Blocks: 8          IO Block: 4096   directory
Device: 259,2    Inode: 21561426    Links: 2
Access: (0755/drwxr-xr-x)  Uid: ( 1000/    nate)   Gid: ( 1000/    nate)
Access: 2024-11-11 08:26:02.696280292 -0600
Modify: 2024-11-11 08:26:02.696280292 -0600
Change: 2024-11-11 08:26:02.696280292 -0600
  Birth: 2024-11-11 08:26:02.696280292 -0600

$ tar --no-overwrite-dir -xf test2.tar
$ stat test
   File: test
   Size: 4096          Blocks: 8          IO Block: 4096   directory
Device: 259,2    Inode: 21561426    Links: 2
Access: (0755/drwxr-xr-x)  Uid: ( 1000/    nate)   Gid: ( 1000/    nate)
Access: 2024-11-11 08:26:02.696280292 -0600
Modify: 2024-11-11 08:23:40.000000000 -0600
Change: 2024-11-11 08:26:17.222659660 -0600
  Birth: 2024-11-11 08:26:02.696280292 -0600
diff --git a/src/extract.c b/src/extract.c
index f741943f..d3205766 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -1134,25 +1134,31 @@ extract_dir (char *file_name, char typeflag)
 			  /* Temporarily change the directory mode to a safe
 			     value, to be able to create files in it, should
 			     the need be.
+           If extracting as non-root user without permission to
+           the target folder, the user assumes responsibility for
+           ensuring workable permissions.
 			  */
-			  mode = safe_dir_mode (&st);
-			  status = fd_chmod (-1, file_name, mode,
-					     AT_SYMLINK_NOFOLLOW, DIRTYPE);
-			  if (status == 0)
-			    {
-			      /* Store the actual directory mode, to be restored
-				 later.
-			      */
-			      current_stat_info.stat = st;
-			      current_mode = mode & ~ current_umask;
-			      current_mode_mask = MODE_RWX;
-			      atflag = AT_SYMLINK_NOFOLLOW;
-			      break;
-			    }
-			  else
-			    {
-			      chmod_error_details (file_name, mode);
-			    }
+              if (we_are_root || st.st_uid == geteuid ())
+              {
+                mode = safe_dir_mode (&st);
+                status = fd_chmod (-1, file_name, mode,
+                            AT_SYMLINK_NOFOLLOW, DIRTYPE);
+                if (status == 0)
+                    {
+                    /* Store the actual directory mode, to be restored
+                    later.
+                    */
+                    current_stat_info.stat = st;
+                    current_mode = mode & ~ current_umask;
+                    current_mode_mask = MODE_RWX;
+                    atflag = AT_SYMLINK_NOFOLLOW;
+                    break;
+                    }
+                else
+                    {
+                    chmod_error_details (file_name, mode);
+                    }
+              }
 			}
 		      break;
 		    }

Reply via email to