Fixed #29027 -- Fixed file_move_safe() crash when moving files with SELinux.

This commit is contained in:
Yuri Konotopov 2022-09-14 16:24:43 +04:00
parent a69b0e9cfe
commit 6d6bf435da
No known key found for this signature in database
GPG Key ID: 6AFB7B50A87DF972
2 changed files with 49 additions and 16 deletions

View File

@ -5,9 +5,8 @@ Move a file in the safest way possible::
>>> file_move_safe("/tmp/old_file", "/tmp/new_file") >>> file_move_safe("/tmp/old_file", "/tmp/new_file")
""" """
import errno
import os import os
from shutil import copystat from shutil import copymode, copystat
from django.core.files import locks from django.core.files import locks
@ -82,12 +81,15 @@ def file_move_safe(
try: try:
copystat(old_file_name, new_file_name) copystat(old_file_name, new_file_name)
except PermissionError as e: except PermissionError:
# Certain filesystems (e.g. CIFS) fail to copy the file's metadata if # Certain filesystems (e.g. CIFS) fail to copy the file's metadata if
# the type of the destination filesystem isn't the same as the source # the type of the destination filesystem isn't the same as the source
# filesystem; ignore that. # filesystem. This also happens with some SELinux-enabled systems.
if e.errno != errno.EPERM: # Ignore that, but try to set basic permissions.
raise try:
copymode(old_file_name, new_file_name)
except PermissionError:
pass
try: try:
os.remove(old_file_name) os.remove(old_file_name)

View File

@ -419,35 +419,66 @@ class FileMoveSafeTests(unittest.TestCase):
os.close(handle_a) os.close(handle_a)
os.close(handle_b) os.close(handle_b)
def test_file_move_copystat_cifs(self): def test_file_move_permissionerror(self):
""" """
file_move_safe() ignores a copystat() EPERM PermissionError. This file_move_safe() ignores PermissionError thrown by copystat() and
happens when the destination filesystem is CIFS, for example. by copymode().
This is raised when the destination filesystem is CIFS, for
example. This also can be raised when relabel is disabled by
SELinux across filesystems.
""" """
copystat_EACCES_error = PermissionError(errno.EACCES, "msg") permission_error = PermissionError(errno.EPERM, "msg")
copystat_EPERM_error = PermissionError(errno.EPERM, "msg") os_error = OSError("msg")
handle_a, self.file_a = tempfile.mkstemp() handle_a, self.file_a = tempfile.mkstemp()
handle_b, self.file_b = tempfile.mkstemp() handle_b, self.file_b = tempfile.mkstemp()
handle_c, self.file_c = tempfile.mkstemp()
try: try:
# This exception is required to reach the copystat() call in # This exception is required to reach the copystat() call in
# file_safe_move(). # file_safe_move().
with mock.patch("django.core.files.move.os.rename", side_effect=OSError()): with mock.patch("django.core.files.move.os.rename", side_effect=OSError()):
# An error besides EPERM isn't ignored. # An error besides PermissionError isn't ignored.
with mock.patch( with mock.patch(
"django.core.files.move.copystat", side_effect=copystat_EACCES_error "django.core.files.move.copystat", side_effect=os_error
): ):
with self.assertRaises(PermissionError): with self.assertRaises(OSError):
file_move_safe(self.file_a, self.file_b, allow_overwrite=True) file_move_safe(self.file_a, self.file_b, allow_overwrite=True)
# EPERM is ignored.
# When copystat() throws PermissionError, copymode() error besides
# PermissionError isn't ignored.
with mock.patch( with mock.patch(
"django.core.files.move.copystat", side_effect=copystat_EPERM_error "django.core.files.move.copystat", side_effect=permission_error
):
with mock.patch(
"django.core.files.move.copymode", side_effect=os_error
):
with self.assertRaises(OSError):
file_move_safe(
self.file_a, self.file_b, allow_overwrite=True
)
# PermissionError raised by copystat() is ignored.
with mock.patch(
"django.core.files.move.copystat",
side_effect=permission_error,
): ):
self.assertIsNone( self.assertIsNone(
file_move_safe(self.file_a, self.file_b, allow_overwrite=True) file_move_safe(self.file_a, self.file_b, allow_overwrite=True)
) )
# PermissionError raised by copymode() is ignored too.
with mock.patch(
"django.core.files.move.copymode",
side_effect=permission_error,
):
self.assertIsNone(
file_move_safe(
self.file_c, self.file_b, allow_overwrite=True
)
)
finally: finally:
os.close(handle_a) os.close(handle_a)
os.close(handle_b) os.close(handle_b)
os.close(handle_c)
class SpooledTempTests(unittest.TestCase): class SpooledTempTests(unittest.TestCase):