paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-10-23 04:52:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

upload: Use normpath when comparing to LOCAL_UPLOADS_DIR.

Browse Source 
This prevents a development-mode-only directory traversal attack,
where the Django development server could be made to respond to
requests for `/user_avatars/../../../../../../etc/passwd`.

The production server is not affected by this vulnerability, as
nginx's configuration sets `PATH_INFO` to `$document_uri`, which is
normalized[^1] -- that is, by the time uwsgi and Django see it, the path
has been percent-decoded once, and all `../` path components have been
applied[^2].

Close this by explicitly normalizing the paths before comparing; the
`LOCAL_UPLOADS_DIR` side is unlikely to require normalization as well,
but is also normalized for consistency.  The failure here is left as
an assertion failure, and not a JsonableError, because it only affects
the development server.

[^1]: https://nginx.org/en/docs/http/ngx_http_core_module.html#var_uri
[^2]: https://nginx.org/en/docs/http/ngx_http_core_module.html#location
This commit is contained in:
Alex Vandiver
2025-10-14 18:24:56 +00:00
committed by Tim Abbott
parent 5d7adcbc00
commit 9815db9811
1 changed files with 2 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
zerver/lib/upload/local.py
View File

@@ -27,7 +27,8 @@ def assert_is_local_storage_path(type: Literal["avatars", "files"], full_path: s
defense in depth.
"""
assert settings.LOCAL_UPLOADS_DIR is not None
type_path = os.path.join(settings.LOCAL_UPLOADS_DIR, type)
type_path = os.path.normpath(os.path.join(settings.LOCAL_UPLOADS_DIR, type))
full_path = os.path.normpath(full_path)
assert os.path.commonpath([type_path, full_path]) == type_path