[ Please Cc me on replies, I am not subscribed to bug-tar@ ]
Hello Nate,
thank you for the observation and patch. To me the new behavior (introduced by
http://git.savannah.gnu.org/cgit/tar.git/commit/?id=14d8fc718f0
in response to
https://www.mail-archive.com/bug-tar@gnu.org/msg05838.html
) looks incorrect as well. But see below:
On Tue, Nov 26, 2024 at 02:50:26PM -0600, Nate Simon wrote:
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.
I would like to know the reason, because when extracting to an existing
directory that is not writable, tar does not make it writable, instead
it fails:
$ chmod a-w .
$ tar -x -f ../test.tar
tar: test: Cannot mkdir: Permission denied
tar: Exiting with failure status due to previous errors
so the user is responsible for having the permissions or appropriate
privileges to write to the directory, and it does not seem to have been
a problem, so why should be existing subdirectories given the
--no-overwrite-dir option behave differently? I feel the code is a bit
too smart.
Therefore I would like to propose an alternative patch that simply
removes the code to change the directory permissions - attached. WDYT?
(Of course the associated test would need changing as well.)
Best regards, Pavel
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;
}
