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;
}