Skip to content

Commit

Permalink
[3.1.x] Fixed CVE-2020-24583, #31921 -- Fixed permissions on intermed…
Browse files Browse the repository at this point in the history
…iate-level static and storage directories on Python 3.7+.

Thanks WhiteSage for the report.

Backport of ea0febbba531a3ecc8c77b570efbfb68ca7155db from master.
  • Loading branch information
felixxm authored and carltongibson committed Sep 1, 2020
1 parent dfa31f8 commit 934430d
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 27 deletions.
6 changes: 3 additions & 3 deletions django/core/files/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,9 @@ def _save(self, name, content):
directory = os.path.dirname(full_path)
try:
if self.directory_permissions_mode is not None:
# os.makedirs applies the global umask, so we reset it,
# for consistency with file_permissions_mode behavior.
old_umask = os.umask(0)
# Set the umask because os.makedirs() doesn't apply the "mode"
# argument to intermediate-level directories.
old_umask = os.umask(0o777 & ~self.directory_permissions_mode)
try:
os.makedirs(directory, self.directory_permissions_mode, exist_ok=True)
finally:
Expand Down
13 changes: 12 additions & 1 deletion docs/releases/2.2.16.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,18 @@ Django 2.2.16 release notes

*Expected September 1, 2020*

Django 2.2.16 fixes two data loss bugs in 2.2.15.
Django 2.2.16 fixes a security issue and two data loss bugs in 2.2.15.

CVE-2020-24583: Incorrect permissions on intermediate-level directories on Python 3.7+
======================================================================================

On Python 3.7+, :setting:`FILE_UPLOAD_DIRECTORY_PERMISSIONS` mode was not
applied to intermediate-level directories created in the process of uploading
files and to intermediate-level collected static directories when using the
:djadmin:`collectstatic` management command.

You should review and manually fix permissions on existing intermediate-level
directories.

Bugfixes
========
Expand Down
13 changes: 12 additions & 1 deletion docs/releases/3.0.10.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,18 @@ Django 3.0.10 release notes

*Expected September 1, 2020*

Django 3.0.10 fixes two data loss bugs in 3.0.9.
Django 3.0.10 fixes a security issue and two data loss bugs in 3.0.9.

CVE-2020-24583: Incorrect permissions on intermediate-level directories on Python 3.7+
======================================================================================

On Python 3.7+, :setting:`FILE_UPLOAD_DIRECTORY_PERMISSIONS` mode was not
applied to intermediate-level directories created in the process of uploading
files and to intermediate-level collected static directories when using the
:djadmin:`collectstatic` management command.

You should review and manually fix permissions on existing intermediate-level
directories.

Bugfixes
========
Expand Down
13 changes: 12 additions & 1 deletion docs/releases/3.1.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,18 @@ Django 3.1.1 release notes

*Expected September 1, 2020*

Django 3.1.1 fixes several bugs in 3.1.
Django 3.1.1 fixes a security issue and several bugs in 3.1.

CVE-2020-24583: Incorrect permissions on intermediate-level directories on Python 3.7+
======================================================================================

On Python 3.7+, :setting:`FILE_UPLOAD_DIRECTORY_PERMISSIONS` mode was not
applied to intermediate-level directories created in the process of uploading
files and to intermediate-level collected static directories when using the
:djadmin:`collectstatic` management command.

You should review and manually fix permissions on existing intermediate-level
directories.

Bugfixes
========
Expand Down
15 changes: 9 additions & 6 deletions tests/file_storage/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -972,16 +972,19 @@ def test_file_upload_default_permissions(self):
@override_settings(FILE_UPLOAD_DIRECTORY_PERMISSIONS=0o765)
def test_file_upload_directory_permissions(self):
self.storage = FileSystemStorage(self.storage_dir)
name = self.storage.save("the_directory/the_file", ContentFile("data"))
dir_mode = os.stat(os.path.dirname(self.storage.path(name)))[0] & 0o777
self.assertEqual(dir_mode, 0o765)
name = self.storage.save('the_directory/subdir/the_file', ContentFile('data'))
file_path = Path(self.storage.path(name))
self.assertEqual(file_path.parent.stat().st_mode & 0o777, 0o765)
self.assertEqual(file_path.parent.parent.stat().st_mode & 0o777, 0o765)

@override_settings(FILE_UPLOAD_DIRECTORY_PERMISSIONS=None)
def test_file_upload_directory_default_permissions(self):
self.storage = FileSystemStorage(self.storage_dir)
name = self.storage.save("the_directory/the_file", ContentFile("data"))
dir_mode = os.stat(os.path.dirname(self.storage.path(name)))[0] & 0o777
self.assertEqual(dir_mode, 0o777 & ~self.umask)
name = self.storage.save('the_directory/subdir/the_file', ContentFile('data'))
file_path = Path(self.storage.path(name))
expected_mode = 0o777 & ~self.umask
self.assertEqual(file_path.parent.stat().st_mode & 0o777, expected_mode)
self.assertEqual(file_path.parent.parent.stat().st_mode & 0o777, expected_mode)


class FileStoragePathParsing(SimpleTestCase):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
html {height: 100%;}
52 changes: 37 additions & 15 deletions tests/staticfiles_tests/test_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import tempfile
import unittest
from io import StringIO
from pathlib import Path
from unittest import mock

from django.conf import settings
Expand Down Expand Up @@ -457,25 +458,39 @@ def run_collectstatic(self, **kwargs):
)
def test_collect_static_files_permissions(self):
call_command('collectstatic', **self.command_params)
test_file = os.path.join(settings.STATIC_ROOT, "test.txt")
test_dir = os.path.join(settings.STATIC_ROOT, "subdir")
file_mode = os.stat(test_file)[0] & 0o777
dir_mode = os.stat(test_dir)[0] & 0o777
static_root = Path(settings.STATIC_ROOT)
test_file = static_root / 'test.txt'
file_mode = test_file.stat().st_mode & 0o777
self.assertEqual(file_mode, 0o655)
self.assertEqual(dir_mode, 0o765)
tests = [
static_root / 'subdir',
static_root / 'nested',
static_root / 'nested' / 'css',
]
for directory in tests:
with self.subTest(directory=directory):
dir_mode = directory.stat().st_mode & 0o777
self.assertEqual(dir_mode, 0o765)

@override_settings(
FILE_UPLOAD_PERMISSIONS=None,
FILE_UPLOAD_DIRECTORY_PERMISSIONS=None,
)
def test_collect_static_files_default_permissions(self):
call_command('collectstatic', **self.command_params)
test_file = os.path.join(settings.STATIC_ROOT, "test.txt")
test_dir = os.path.join(settings.STATIC_ROOT, "subdir")
file_mode = os.stat(test_file)[0] & 0o777
dir_mode = os.stat(test_dir)[0] & 0o777
static_root = Path(settings.STATIC_ROOT)
test_file = static_root / 'test.txt'
file_mode = test_file.stat().st_mode & 0o777
self.assertEqual(file_mode, 0o666 & ~self.umask)
self.assertEqual(dir_mode, 0o777 & ~self.umask)
tests = [
static_root / 'subdir',
static_root / 'nested',
static_root / 'nested' / 'css',
]
for directory in tests:
with self.subTest(directory=directory):
dir_mode = directory.stat().st_mode & 0o777
self.assertEqual(dir_mode, 0o777 & ~self.umask)

@override_settings(
FILE_UPLOAD_PERMISSIONS=0o655,
Expand All @@ -484,12 +499,19 @@ def test_collect_static_files_default_permissions(self):
)
def test_collect_static_files_subclass_of_static_storage(self):
call_command('collectstatic', **self.command_params)
test_file = os.path.join(settings.STATIC_ROOT, "test.txt")
test_dir = os.path.join(settings.STATIC_ROOT, "subdir")
file_mode = os.stat(test_file)[0] & 0o777
dir_mode = os.stat(test_dir)[0] & 0o777
static_root = Path(settings.STATIC_ROOT)
test_file = static_root / 'test.txt'
file_mode = test_file.stat().st_mode & 0o777
self.assertEqual(file_mode, 0o640)
self.assertEqual(dir_mode, 0o740)
tests = [
static_root / 'subdir',
static_root / 'nested',
static_root / 'nested' / 'css',
]
for directory in tests:
with self.subTest(directory=directory):
dir_mode = directory.stat().st_mode & 0o777
self.assertEqual(dir_mode, 0o740)


@override_settings(
Expand Down

0 comments on commit 934430d

Please sign in to comment.