Skip to content

Commit c28e39a

Browse files
authored
Merge pull request from GHSA-v7x4-rhpg-3p2r
Use safe_open instead of open
1 parent 448f34e commit c28e39a

File tree

4 files changed

+49
-5
lines changed

4 files changed

+49
-5
lines changed

readthedocs/core/tests/test_filesystem_utils.py

+21-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
from readthedocs.core.utils.filesystem import safe_copytree, safe_open, safe_rmtree
99
from readthedocs.doc_builder.exceptions import (
10-
FileIsNotRegularFile,
10+
FileTooLarge,
1111
SymlinkOutsideBasePath,
1212
UnsupportedSymlinkFileError,
1313
)
@@ -103,6 +103,26 @@ def test_open(self):
103103
)
104104
self.assertIsNotNone(context_manager)
105105

106+
def test_open_large_file(self):
107+
root_directory = Path(mkdtemp())
108+
docroot_path = root_directory
109+
file_a = root_directory / "test.txt"
110+
file_a.write_bytes(b"0" * (1024 * 2))
111+
112+
with override_settings(DOCROOT=docroot_path):
113+
with pytest.raises(FileTooLarge):
114+
safe_open(file_a, max_size_bytes=1024)
115+
116+
def test_write_file(self):
117+
root_directory = Path(mkdtemp())
118+
docroot_path = root_directory
119+
file_a = root_directory / "test.txt"
120+
121+
with override_settings(DOCROOT=docroot_path):
122+
with safe_open(file_a, mode="w") as f:
123+
f.write("Hello World")
124+
self.assertEqual(file_a.read_text(), "Hello World")
125+
106126
def test_open_outside_docroot(self):
107127
root_directory = Path(mkdtemp())
108128
docroot_path = Path(mkdtemp())

readthedocs/core/utils/filesystem.py

+19-2
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,14 @@
1414

1515
from readthedocs.doc_builder.exceptions import (
1616
FileIsNotRegularFile,
17+
FileTooLarge,
1718
SymlinkOutsideBasePath,
1819
UnsupportedSymlinkFileError,
1920
)
2021

2122
log = structlog.get_logger(__name__)
2223

24+
MAX_FILE_SIZE_BYTES = 1024 * 1024 * 1024 * 1 # 1 GB
2325

2426
def _assert_path_is_inside_docroot(path):
2527
resolved_path = path.absolute().resolve()
@@ -32,13 +34,21 @@ def _assert_path_is_inside_docroot(path):
3234
raise SuspiciousFileOperation(path)
3335

3436

35-
def safe_open(path, *args, allow_symlinks=False, base_path=None, **kwargs):
37+
def safe_open(
38+
path,
39+
*args,
40+
allow_symlinks=False,
41+
base_path=None,
42+
max_size_bytes=MAX_FILE_SIZE_BYTES,
43+
**kwargs
44+
):
3645
"""
3746
Wrapper around path.open() to check for symlinks.
3847
3948
- Checks for symlinks to avoid symlink following vulnerabilities
4049
like GHSA-368m-86q9-m99w.
4150
- Checks that files aren't out of the DOCROOT directory.
51+
- Checks that files aren't too large.
4252
4353
:param allow_symlinks: If `False` and the path is a symlink, raise `FileIsSymlink`
4454
This prevents reading the contents of other files users shouldn't have access to.
@@ -47,6 +57,7 @@ def safe_open(path, *args, allow_symlinks=False, base_path=None, **kwargs):
4757
(usually the directory where the project was cloned).
4858
It must be given if `allow_symlinks` is set to `True`.
4959
This prevents path traversal attacks (even when using symlinks).
60+
:param max_size_bytes: Maximum file size allowed in bytes when reading a file.
5061
5162
The extra *args and **kwargs will be passed to the open() method.
5263
@@ -56,7 +67,7 @@ def safe_open(path, *args, allow_symlinks=False, base_path=None, **kwargs):
5667
Users shouldn't be able to change files while this operation is done.
5768
"""
5869
if allow_symlinks and not base_path:
59-
raise ValueError("base_path mut be given if symlinks are allowed.")
70+
raise ValueError("base_path must be given if symlinks are allowed.")
6071

6172
path = Path(path).absolute()
6273

@@ -74,6 +85,12 @@ def safe_open(path, *args, allow_symlinks=False, base_path=None, **kwargs):
7485
# Expand symlinks.
7586
resolved_path = path.resolve()
7687

88+
if resolved_path.exists():
89+
file_size = resolved_path.stat().st_size
90+
if file_size > max_size_bytes:
91+
log.info("File is too large.", size_bytes=file_size)
92+
raise FileTooLarge(path)
93+
7794
if allow_symlinks and base_path:
7895
base_path = Path(base_path).absolute()
7996
if not resolved_path.is_relative_to(base_path):

readthedocs/doc_builder/backends/mkdocs.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ def append_conf(self):
214214
docs_dir=os.path.relpath(docs_path, self.project_path),
215215
mkdocs_config=user_config,
216216
)
217-
with open(
217+
with safe_open(
218218
os.path.join(docs_path, "readthedocs-data.js"), "w", encoding="utf-8"
219219
) as f:
220220
f.write(rtd_data)
@@ -231,7 +231,7 @@ def append_conf(self):
231231
user_config['theme'] = self.DEFAULT_THEME_NAME
232232

233233
# Write the modified mkdocs configuration
234-
with open(self.yaml_file, "w", encoding="utf-8") as f:
234+
with safe_open(self.yaml_file, "w", encoding="utf-8") as f:
235235
yaml_dump_safely(
236236
user_config,
237237
f,

readthedocs/doc_builder/exceptions.py

+7
Original file line numberDiff line numberDiff line change
@@ -140,3 +140,10 @@ class FileIsNotRegularFile(UnsupportedSymlinkFileError):
140140

141141
class SymlinkOutsideBasePath(UnsupportedSymlinkFileError):
142142
pass
143+
144+
145+
class FileTooLarge(BuildUserError):
146+
message = gettext_noop(
147+
"A file from your build process is too large to be processed by Read the Docs. "
148+
"Please ensure no files generated are larger than 1GB."
149+
)

0 commit comments

Comments
 (0)