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

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)