diff --git a/Doc/library/shutil.rst b/Doc/library/shutil.rst --- a/Doc/library/shutil.rst +++ b/Doc/library/shutil.rst @@ -73,6 +73,10 @@ Raise :exc:`SameFileError` instead of :exc:`Error`. Since the former is a subclass of the latter, this change is backward compatible. + .. versionchanged:: XXX + Don't preserve setuid, setgid and sticky bits for security reasons, see + :func:`copymode` + .. exception:: SameFileError @@ -96,6 +100,11 @@ .. versionchanged:: 3.3 Added *follow_symlinks* argument. + .. versionchanged:: XXX + Don't preserve setuid, setgid and sticky bits for security reasons, see + :func:`copymode` + + .. function:: copystat(src, dst, *, follow_symlinks=True) Copy the permission bits, last access time, last modification time, and @@ -137,9 +146,15 @@ Please see :data:`os.supports_follow_symlinks` for more information. + The setuid, setgid and sticky bits aren't copied to prevent privilege + escalations. + .. versionchanged:: 3.3 Added *follow_symlinks* argument and support for Linux extended attributes. + .. versionchanged:: XXX + Don't preserve setuid, setgid and sticky bits for security reasons. + .. function:: copy(src, dst, *, follow_symlinks=True) Copies the file *src* to the file or directory *dst*. *src* and *dst* diff --git a/Lib/shutil.py b/Lib/shutil.py --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -109,6 +109,9 @@ copyfileobj(fsrc, fdst) return dst +# complementary bit mask of harmful bits (suid, sgid, sticky) +_REMOVE_HARMFUL_MASK = ~ (stat.S_ISUID | stat.S_ISGID | stat.S_ISVTX) + def copymode(src, dst, *, follow_symlinks=True): """Copy mode bits from src to dst. @@ -128,7 +131,10 @@ return st = stat_func(src) - chmod_func(dst, stat.S_IMODE(st.st_mode)) + mode = stat.S_IMODE(st.st_mode) + # remove setuid, setgid and sticky bits + mode &= _REMOVE_HARMFUL_MASK + chmod_func(dst, mode) if hasattr(os, 'listxattr'): def _copyxattr(src, dst, *, follow_symlinks=True): @@ -184,6 +190,8 @@ st = lookup("stat")(src, follow_symlinks=follow) mode = stat.S_IMODE(st.st_mode) + # remove setuid, setgid and sticky bits + mode &= _REMOVE_HARMFUL_MASK lookup("utime")(dst, ns=(st.st_atime_ns, st.st_mtime_ns), follow_symlinks=follow) try: diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -1287,6 +1287,25 @@ rv = shutil.copytree(src_dir, dst_dir) self.assertEqual(['foo'], os.listdir(rv)) + @unittest.skipUnless(os.name == "posix", "Requires POSIX compatible OS") + def test_copy_remove_setuid(self): + src_dir = self.mkdtemp() + src_file = os.path.join(src_dir, 'foo') + write_file(src_file, 'foo') + dst_file = os.path.join(src_dir, 'bar') + harmful_mode = stat.S_IRUSR | stat.S_IXUSR | stat.S_ISUID + harmless_mode = stat.S_IRUSR | stat.S_IXUSR + + # set mode and verify + os.chmod(src_file, harmful_mode) + mode = stat.S_IMODE(os.stat(src_file).st_mode) + self.assertTrue(oct(mode), oct(harmful_mode)) + + # check that copy does not preserve harmful bits + shutil.copy(src_file, dst_file) + mode = stat.S_IMODE(os.stat(dst_file).st_mode) + self.assertEqual(oct(mode), oct(harmless_mode)) + class TestWhich(unittest.TestCase):