Package: p7zip Version: 4.43~dfsg.1-2 Severity: important Tags: patch *** Please type your report below this line ***
p7zip interperets files read-only file permissions as 444, read-write as 666, and executable as 777 (unsure of full logic but you get the idea). Similarly, read-write directories are interpereted as 777. Apparently the format has the posssibility to store unix permissions, but most files will originate on windows and be handled in this way. All of these do not respect the umask. This means that most unpacked .7z files will create directories that are world-readable, which is pretty undesirable. World readable directories can defeat default controls on who can write to what filesystems, and allow users to create files in private storage areas, as well as directories in same. Creating world-writable directories against the umask setting may lead to users with foreign-owned unwritable directories in their private storage areas, which may lead to a good deal of confusion for them, as well as the inability to remove or properly manage their home space. More simply, p7zip does not respect the umask in a way that is prone to creating usability and/or administration problems. Essentially the program is a rough port from naive win32 code, that creates files then applies permissions after-the-fact with chmod, avoiding the umask. I have not checked to see what default permissions the files are created with, there may be a race where the files are writable before the permissions are applied. This patch enforces the umask against all files, regardless of the permissions and modes in the archive. You may want to consider not applying the umask in the case that full unix permissions are acquired from the archive. Probably the application or nonapplication of umask in this case should be user controllable. I'm not sure what the right default is. Perhaps one should look to tar. In short, I simply feel this patch makes the behavior of p7zip "less bad". But C++ is really not my forte, and the structure of the program is more than I wish to seriously wrestle with at this time. I have alerted upstream, although I have not specifically filed a bug, as I'm not exactly sure how to classify it, and also because I knew a lot less about the issue on initially raising it. Here is a link: http://sourceforge.net/forum/message.php?msg_id=4194787 Patch: diff -Naur p7zip-4.43~dfsg.1.orig/Windows/FileDir.cpp p7zip-4.43~dfsg.1/Windows/FileDir.cpp --- p7zip-4.43~dfsg.1.orig/Windows/FileDir.cpp 2007-03-07 02:34:25.000000000 -0800 +++ p7zip-4.43~dfsg.1/Windows/FileDir.cpp 2007-03-07 03:22:18.000000000 -0800 @@ -18,6 +18,7 @@ #include <sys/stat.h> // mkdir #include <sys/types.h> +#include <sys/stat.h> #include <fcntl.h> #include <utime.h> @@ -329,6 +330,13 @@ } } + mode_t user_mask = umask(0); + // This is a race condition. Any parallel thread or forked process at + // this point gets an undesirable umask. The correct fix is to remove + // all chmod calls from the code, and determine permissions before files + // are open()ed or mkdir()ed. + umask(user_mask); + if (fileAttributes & FILE_ATTRIBUTE_UNIX_EXTENSION) { stat_info.st_mode = fileAttributes >> 16; #ifdef HAVE_LSTAT @@ -340,11 +348,11 @@ } else #endif if (S_ISREG(stat_info.st_mode)) { - chmod(name,stat_info.st_mode); + chmod(name,stat_info.st_mode & ~user_mask); } else if (S_ISDIR(stat_info.st_mode)) { // user/7za must be able to create files in this directory stat_info.st_mode |= (S_IRUSR | S_IWUSR | S_IXUSR); - chmod(name,stat_info.st_mode); + chmod(name,stat_info.st_mode & ~user_mask); } #ifdef HAVE_LSTAT } else if (!S_ISLNK(stat_info.st_mode)) { @@ -362,7 +370,7 @@ stat_info.st_mode |= (0600 | ((stat_info.st_mode & 044) >> 1)) ; } - chmod(name,stat_info.st_mode); + chmod(name,stat_info.st_mode & ~user_mask); } TRACEN((printf("MySetFileAttributes(%s,%d) : true\n",name,fileAttributes))) diff -Naur p7zip-4.43~dfsg.1.orig/myWindows/myAddExeFlag.cpp p7zip-4.43~dfsg.1/myWindows/myAddExeFlag.cpp --- p7zip-4.43~dfsg.1.orig/myWindows/myAddExeFlag.cpp 2007-03-07 02:34:25.000000000 -0800 +++ p7zip-4.43~dfsg.1/myWindows/myAddExeFlag.cpp 2007-03-07 02:54:26.000000000 -0800 @@ -13,9 +13,13 @@ void myAddExeFlag(LPCTSTR filename) { + struct stat fileinfo; const char * name = nameWindowToUnix(filename); // printf("myAddExeFlag(%s)\n",name); - chmod(name,0777); + // should detect error, but this interface gives no capability to do so + stat(name, &fileinfo); + mode_t filemode = fileinfo.st_mode; + chmod(name, filemode | S_IXUSR | S_IXGRP | S_IXOTH); } void myAddExeFlag(const UString &u_name) -- System Information: Debian Release: 4.0 APT prefers testing APT policy: (990, 'testing') Architecture: amd64 (x86_64) Shell: /bin/sh linked to /bin/dash Kernel: Linux 2.6.19.2-jsr1 Locale: LANG=en_US.iso88591, LC_CTYPE=en_US.iso88591 (charmap=ISO-8859-1) Versions of packages p7zip depends on: ii libc6 2.3.6.ds1-13 GNU C Library: Shared libraries ii libgcc1 1:4.1.1-21 GCC support library ii libstdc++6 4.1.1-21 The GNU Standard C++ Library v3 Versions of packages p7zip recommends: pn p7zip-full <none> (no description available) -- no debconf information -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
