티스토리 수익 글 보기

티스토리 수익 글 보기

[2.2.x] Fixed CVE-2020-24583, #31921 — Fixed permissions on intermed… · django/django@375657a · GitHub
Skip to content
/ django Public

Commit 375657a

Browse files
felixxmfelixxm
committed
[2.2.x] Fixed CVE-2020-24583, #31921 — Fixed permissions on intermediate-level static and storage directories on Python 3.7+.
Thanks WhiteSage for the report. Backport of ea0febbba531a3ecc8c77b570efbfb68ca7155db from master.
1 parent dc39e62 commit 375657a

File tree

5 files changed

+63
–25
lines changed

5 files changed

+63
–25
lines changed

‎django/core/files/storage.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,9 +231,9 @@ def _save(self, name, content):
231231
if not os.path.exists(directory):
232232
try:
233233
if self.directory_permissions_mode is not None:
234-
# os.makedirs applies the global umask, so we reset it,
235-
# for consistency with file_permissions_mode behavior.
236-
old_umask = os.umask(0)
234+
# Set the umask because os.makedirs() doesn't apply the "mode"
235+
# argument to intermediate-level directories.
236+
old_umask = os.umask(0o777 & ~self.directory_permissions_mode)
237237
try:
238238
os.makedirs(directory, self.directory_permissions_mode)
239239
finally:

‎docs/releases/2.2.16.txt‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,18 @@ Django 2.2.16 release notes
44

55
*Expected September 1, 2020*
66

7-
Django 2.2.16 fixes two data loss bugs in 2.2.15.
7+
Django 2.2.16 fixes a security issue and two data loss bugs in 2.2.15.
8+
9+
CVE-2020-24583: Incorrect permissions on intermediate-level directories on Python 3.7+
10+
======================================================================================
11+
12+
On Python 3.7+, :setting:`FILE_UPLOAD_DIRECTORY_PERMISSIONS` mode was not
13+
applied to intermediate-level directories created in the process of uploading
14+
files and to intermediate-level collected static directories when using the
15+
:djadmin:`collectstatic` management command.
16+
17+
You should review and manually fix permissions on existing intermediate-level
18+
directories.
819

920
Bugfixes
1021
========

‎tests/file_storage/tests.py‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import unittest
88
from datetime import datetime, timedelta
99
from io import StringIO
10+
from pathlib import Path
1011
from urllib.request import urlopen
1112

1213
from django.core.cache import cache
@@ -901,16 +902,19 @@ def test_file_upload_default_permissions(self):
901902
@override_settings(FILE_UPLOAD_DIRECTORY_PERMISSIONS=0o765)
902903
def test_file_upload_directory_permissions(self):
903904
self.storage = FileSystemStorage(self.storage_dir)
904-
name = self.storage.save("the_directory/the_file", ContentFile("data"))
905-
dir_mode = os.stat(os.path.dirname(self.storage.path(name)))[0] & 0o777
906-
self.assertEqual(dir_mode, 0o765)
905+
name = self.storage.save('the_directory/subdir/the_file', ContentFile('data'))
906+
file_path = Path(self.storage.path(name))
907+
self.assertEqual(file_path.parent.stat().st_mode & 0o777, 0o765)
908+
self.assertEqual(file_path.parent.parent.stat().st_mode & 0o777, 0o765)
907909

908910
@override_settings(FILE_UPLOAD_DIRECTORY_PERMISSIONS=None)
909911
def test_file_upload_directory_default_permissions(self):
910912
self.storage = FileSystemStorage(self.storage_dir)
911-
name = self.storage.save("the_directory/the_file", ContentFile("data"))
912-
dir_mode = os.stat(os.path.dirname(self.storage.path(name)))[0] & 0o777
913-
self.assertEqual(dir_mode, 0o777 & ~self.umask)
913+
name = self.storage.save('the_directory/subdir/the_file', ContentFile('data'))
914+
file_path = Path(self.storage.path(name))
915+
expected_mode = 0o777 & ~self.umask
916+
self.assertEqual(file_path.parent.stat().st_mode & 0o777, expected_mode)
917+
self.assertEqual(file_path.parent.parent.stat().st_mode & 0o777, expected_mode)
914918

915919

916920
class FileStoragePathParsing(SimpleTestCase):

‎tests/staticfiles_tests/project/documents/nested/css/base.css‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
html {height: 100%;}

‎tests/staticfiles_tests/test_storage.py‎

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import tempfile
55
import unittest
66
from io import StringIO
7+
from pathlib import Path
78

89
from django.conf import settings
910
from django.contrib.staticfiles import finders, storage
@@ -508,25 +509,39 @@ def run_collectstatic(self, **kwargs):
508509
)
509510
def test_collect_static_files_permissions(self):
510511
call_command('collectstatic', **self.command_params)
511-
test_file = os.path.join(settings.STATIC_ROOT, "test.txt")
512-
test_dir = os.path.join(settings.STATIC_ROOT, "subdir")
513-
file_mode = os.stat(test_file)[0] & 0o777
514-
dir_mode = os.stat(test_dir)[0] & 0o777
512+
static_root = Path(settings.STATIC_ROOT)
513+
test_file = static_root / 'test.txt'
514+
file_mode = test_file.stat().st_mode & 0o777
515515
self.assertEqual(file_mode, 0o655)
516-
self.assertEqual(dir_mode, 0o765)
516+
tests = [
517+
static_root / 'subdir',
518+
static_root / 'nested',
519+
static_root / 'nested' / 'css',
520+
]
521+
for directory in tests:
522+
with self.subTest(directory=directory):
523+
dir_mode = directory.stat().st_mode & 0o777
524+
self.assertEqual(dir_mode, 0o765)
517525

518526
@override_settings(
519527
FILE_UPLOAD_PERMISSIONS=None,
520528
FILE_UPLOAD_DIRECTORY_PERMISSIONS=None,
521529
)
522530
def test_collect_static_files_default_permissions(self):
523531
call_command('collectstatic', **self.command_params)
524-
test_file = os.path.join(settings.STATIC_ROOT, "test.txt")
525-
test_dir = os.path.join(settings.STATIC_ROOT, "subdir")
526-
file_mode = os.stat(test_file)[0] & 0o777
527-
dir_mode = os.stat(test_dir)[0] & 0o777
532+
static_root = Path(settings.STATIC_ROOT)
533+
test_file = static_root / 'test.txt'
534+
file_mode = test_file.stat().st_mode & 0o777
528535
self.assertEqual(file_mode, 0o666 & ~self.umask)
529-
self.assertEqual(dir_mode, 0o777 & ~self.umask)
536+
tests = [
537+
static_root / 'subdir',
538+
static_root / 'nested',
539+
static_root / 'nested' / 'css',
540+
]
541+
for directory in tests:
542+
with self.subTest(directory=directory):
543+
dir_mode = directory.stat().st_mode & 0o777
544+
self.assertEqual(dir_mode, 0o777 & ~self.umask)
530545

531546
@override_settings(
532547
FILE_UPLOAD_PERMISSIONS=0o655,
@@ -535,12 +550,19 @@ def test_collect_static_files_default_permissions(self):
535550
)
536551
def test_collect_static_files_subclass_of_static_storage(self):
537552
call_command('collectstatic', **self.command_params)
538-
test_file = os.path.join(settings.STATIC_ROOT, "test.txt")
539-
test_dir = os.path.join(settings.STATIC_ROOT, "subdir")
540-
file_mode = os.stat(test_file)[0] & 0o777
541-
dir_mode = os.stat(test_dir)[0] & 0o777
553+
static_root = Path(settings.STATIC_ROOT)
554+
test_file = static_root / 'test.txt'
555+
file_mode = test_file.stat().st_mode & 0o777
542556
self.assertEqual(file_mode, 0o640)
543-
self.assertEqual(dir_mode, 0o740)
557+
tests = [
558+
static_root / 'subdir',
559+
static_root / 'nested',
560+
static_root / 'nested' / 'css',
561+
]
562+
for directory in tests:
563+
with self.subTest(directory=directory):
564+
dir_mode = directory.stat().st_mode & 0o777
565+
self.assertEqual(dir_mode, 0o740)
544566

545567

546568
@override_settings(

0 commit comments

Comments
 (0)