summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Payerle <payerle@umd.edu>2020-06-23 20:50:19 -0400
committerGitHub <noreply@github.com>2020-06-23 19:50:19 -0500
commitfd710fc93eee64e3ecd432d902a8bb6b61354451 (patch)
tree6be63c7c0f21730555cb92075a50546c2a5f4f01
parente74c8e71ccfc86ea63446dff5517afcdd286bae0 (diff)
downloadspack-fd710fc93eee64e3ecd432d902a8bb6b61354451.tar.gz
spack-fd710fc93eee64e3ecd432d902a8bb6b61354451.tar.bz2
spack-fd710fc93eee64e3ecd432d902a8bb6b61354451.tar.xz
spack-fd710fc93eee64e3ecd432d902a8bb6b61354451.zip
Some minor fixes to set_permissions() in file_permissions.py (#17020)
* Some minor fixes to set_permissions() in file_permissions.py The set_permissions() routine claims to prevent users from creating world writable suid binaries. However, it seems to only be checking for/preventing group writable suid binaries. This patch modifies the routine to check for both world and group writable suid binaries, and complain appropriately. * permissions.py: Add test to check blocks world writable SUID files The original test_chmod_rejects_group_writable_suid tested that the set_permissions() function in lib/spack/spack/util/file_permissions.py would raise an exception if changed permission on a file with both SUID and SGID plus sticky bits is chmod-ed to g+rwx and o+rwx. I have modified so that more narrowly tests a file with SUID (and no SGID or sticky bit) set is chmod-ed to g+w. I have added a second test test_chmod_rejects_world_writable_suid that checks that exception is raised if an SUID file is chmod-ed to o+w * file_permissions.py: Raise exception when try to make sgid file world writable Updated set_permissions() in file_permissions.py to also raise an exception if try to make an SGID file world writable. And added corresponding unit test as well. * Remove debugging prints from permissions.py
-rw-r--r--lib/spack/spack/test/permissions.py24
-rw-r--r--lib/spack/spack/util/file_permissions.py17
2 files changed, 35 insertions, 6 deletions
diff --git a/lib/spack/spack/test/permissions.py b/lib/spack/spack/test/permissions.py
index fa83eb37ff..26974c8096 100644
--- a/lib/spack/spack/test/permissions.py
+++ b/lib/spack/spack/test/permissions.py
@@ -27,9 +27,29 @@ def test_chmod_real_entries_ignores_suid_sgid(tmpdir):
def test_chmod_rejects_group_writable_suid(tmpdir):
path = str(tmpdir.join('file').ensure())
- mode = stat.S_ISUID | stat.S_ISGID | stat.S_ISVTX
+ mode = stat.S_ISUID
+ fs.chmod_x(path, mode)
+
+ perms = stat.S_IWGRP
+ with pytest.raises(InvalidPermissionsError):
+ set_permissions(path, perms)
+
+
+def test_chmod_rejects_world_writable_suid(tmpdir):
+ path = str(tmpdir.join('file').ensure())
+ mode = stat.S_ISUID
+ fs.chmod_x(path, mode)
+
+ perms = stat.S_IWOTH
+ with pytest.raises(InvalidPermissionsError):
+ set_permissions(path, perms)
+
+
+def test_chmod_rejects_world_writable_sgid(tmpdir):
+ path = str(tmpdir.join('file').ensure())
+ mode = stat.S_ISGID
fs.chmod_x(path, mode)
- perms = stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO
+ perms = stat.S_IWOTH
with pytest.raises(InvalidPermissionsError):
set_permissions(path, perms)
diff --git a/lib/spack/spack/util/file_permissions.py b/lib/spack/spack/util/file_permissions.py
index d94b74f51e..b133d2569e 100644
--- a/lib/spack/spack/util/file_permissions.py
+++ b/lib/spack/spack/util/file_permissions.py
@@ -25,10 +25,19 @@ def set_permissions(path, perms, group=None):
# Preserve higher-order bits of file permissions
perms |= os.stat(path).st_mode & (st.S_ISUID | st.S_ISGID | st.S_ISVTX)
- # Do not let users create world writable suid binaries
- if perms & st.S_ISUID and perms & st.S_IWGRP:
- raise InvalidPermissionsError(
- "Attepting to set suid with world writable")
+ # Do not let users create world/group writable suid binaries
+ if perms & st.S_ISUID:
+ if perms & st.S_IWOTH:
+ raise InvalidPermissionsError(
+ "Attempting to set suid with world writable")
+ if perms & st.S_IWGRP:
+ raise InvalidPermissionsError(
+ "Attempting to set suid with group writable")
+ # Or world writable sgid binaries
+ if perms & st.S_ISGID:
+ if perms & st.S_IWOTH:
+ raise InvalidPermissionsError(
+ "Attempting to set sgid with world writable")
fs.chmod_x(path, perms)