nathan/nixpkgs
1
0
Fork 0
mirror of https://github.com/NixOS/nixpkgs.git synced 2026-08-25 17:55:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

python310Packages.aiohttp: add patch for CVE-2024-23334

Browse Source
This commit is contained in:
Robert Scott
2024-03-20 19:28:34 +00:00
parent bf1829609b
commit 003f8ccc46
2 changed files with 204 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

203
pkgs/development/python-modules/aiohttp/3.8.6-CVE-2024-23334.patch Normal file
View File

@@ -0,0 +1,203 @@
Based on upstream 1c335944d6a8b1298baf179b7c0b3069f10c514b with a couple
of type hints removed from the added tests because they refer to type
aliases that don't exist in 3.8.6
diff --git a/CHANGES/8079.bugfix.rst b/CHANGES/8079.bugfix.rst
new file mode 100644
index 00000000..57bc8bfe
--- /dev/null
+++ b/CHANGES/8079.bugfix.rst
@@ -0,0 +1 @@
+Improved validation of paths for static resources -- by :user:`bdraco`.
diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py
index 5942e355..e8a8023e 100644
--- a/aiohttp/web_urldispatcher.py
+++ b/aiohttp/web_urldispatcher.py
@@ -593,9 +593,14 @@ class StaticResource(PrefixResource):
url = url / filename
if append_version:
+ unresolved_path = self._directory.joinpath(filename)
try:
- filepath = self._directory.joinpath(filename).resolve()
- if not self._follow_symlinks:
+ if self._follow_symlinks:
+ normalized_path = Path(os.path.normpath(unresolved_path))
+ normalized_path.relative_to(self._directory)
+ filepath = normalized_path.resolve()
+ else:
+ filepath = unresolved_path.resolve()
filepath.relative_to(self._directory)
except (ValueError, FileNotFoundError):
# ValueError for case when path point to symlink
@@ -660,8 +665,13 @@ class StaticResource(PrefixResource):
# /static/\\machine_name\c$ or /static/D:\path
# where the static dir is totally different
raise HTTPForbidden()
- filepath = self._directory.joinpath(filename).resolve()
- if not self._follow_symlinks:
+ unresolved_path = self._directory.joinpath(filename)
+ if self._follow_symlinks:
+ normalized_path = Path(os.path.normpath(unresolved_path))
+ normalized_path.relative_to(self._directory)
+ filepath = normalized_path.resolve()
+ else:
+ filepath = unresolved_path.resolve()
filepath.relative_to(self._directory)
except (ValueError, FileNotFoundError) as error:
# relatively safe
diff --git a/docs/web_advanced.rst b/docs/web_advanced.rst
index 3a98b78a..51293970 100644
--- a/docs/web_advanced.rst
+++ b/docs/web_advanced.rst
@@ -136,12 +136,22 @@ instead could be enabled with ``show_index`` parameter set to ``True``::
web.static('/prefix', path_to_static_folder, show_index=True)
-When a symlink from the static directory is accessed, the server responses to
-client with ``HTTP/404 Not Found`` by default. To allow the server to follow
-symlinks, parameter ``follow_symlinks`` should be set to ``True``::
+When a symlink that leads outside the static directory is accessed, the server
+responds to the client with ``HTTP/404 Not Found`` by default. To allow the server to
+follow symlinks that lead outside the static root, the parameter ``follow_symlinks``
+should be set to ``True``::
web.static('/prefix', path_to_static_folder, follow_symlinks=True)
+.. caution::
+
+ Enabling ``follow_symlinks`` can be a security risk, and may lead to
+ a directory transversal attack. You do NOT need this option to follow symlinks
+ which point to somewhere else within the static directory, this option is only
+ used to break out of the security sandbox. Enabling this option is highly
+ discouraged, and only expected to be used for edge cases in a local
+ development setting where remote users do not have access to the server.
+
When you want to enable cache busting,
parameter ``append_version`` can be set to ``True``
diff --git a/docs/web_reference.rst b/docs/web_reference.rst
index a156f47d..b1006764 100644
--- a/docs/web_reference.rst
+++ b/docs/web_reference.rst
@@ -1836,9 +1836,15 @@ Router is any object that implements :class:`~aiohttp.abc.AbstractRouter` interf
by default it's not allowed and HTTP/403 will
be returned on directory access.
- :param bool follow_symlinks: flag for allowing to follow symlinks from
- a directory, by default it's not allowed and
- HTTP/404 will be returned on access.
+ :param bool follow_symlinks: flag for allowing to follow symlinks that lead
+ outside the static root directory, by default it's not allowed and
+ HTTP/404 will be returned on access. Enabling ``follow_symlinks``
+ can be a security risk, and may lead to a directory transversal attack.
+ You do NOT need this option to follow symlinks which point to somewhere
+ else within the static directory, this option is only used to break out
+ of the security sandbox. Enabling this option is highly discouraged,
+ and only expected to be used for edge cases in a local development
+ setting where remote users do not have access to the server.
:param bool append_version: flag for adding file version (hash)
to the url query string, this value will
diff --git a/tests/test_web_urldispatcher.py b/tests/test_web_urldispatcher.py
index f24f451e..6b8381c0 100644
--- a/tests/test_web_urldispatcher.py
+++ b/tests/test_web_urldispatcher.py
@@ -123,6 +123,97 @@ async def test_follow_symlink(tmp_dir_path, aiohttp_client) -> None:
assert (await r.text()) == data
+async def test_follow_symlink_directory_traversal(
+ tmp_path: pathlib.Path, aiohttp_client
+) -> None:
+ # Tests that follow_symlinks does not allow directory transversal
+ data = "private"
+
+ private_file = tmp_path / "private_file"
+ private_file.write_text(data)
+
+ safe_path = tmp_path / "safe_dir"
+ safe_path.mkdir()
+
+ app = web.Application()
+
+ # Register global static route:
+ app.router.add_static("/", str(safe_path), follow_symlinks=True)
+ client = await aiohttp_client(app)
+
+ await client.start_server()
+ # We need to use a raw socket to test this, as the client will normalize
+ # the path before sending it to the server.
+ reader, writer = await asyncio.open_connection(client.host, client.port)
+ writer.write(b"GET /../private_file HTTP/1.1\r\n\r\n")
+ response = await reader.readuntil(b"\r\n\r\n")
+ assert b"404 Not Found" in response
+ writer.close()
+ await writer.wait_closed()
+ await client.close()
+
+
+async def test_follow_symlink_directory_traversal_after_normalization(
+ tmp_path: pathlib.Path, aiohttp_client
+) -> None:
+ # Tests that follow_symlinks does not allow directory transversal
+ # after normalization
+ #
+ # Directory structure
+ # |-- secret_dir
+ # | |-- private_file (should never be accessible)
+ # | |-- symlink_target_dir
+ # | |-- symlink_target_file (should be accessible via the my_symlink symlink)
+ # | |-- sandbox_dir
+ # | |-- my_symlink -> symlink_target_dir
+ #
+ secret_path = tmp_path / "secret_dir"
+ secret_path.mkdir()
+
+ # This file is below the symlink target and should not be reachable
+ private_file = secret_path / "private_file"
+ private_file.write_text("private")
+
+ symlink_target_path = secret_path / "symlink_target_dir"
+ symlink_target_path.mkdir()
+
+ sandbox_path = symlink_target_path / "sandbox_dir"
+ sandbox_path.mkdir()
+
+ # This file should be reachable via the symlink
+ symlink_target_file = symlink_target_path / "symlink_target_file"
+ symlink_target_file.write_text("readable")
+
+ my_symlink_path = sandbox_path / "my_symlink"
+ pathlib.Path(str(my_symlink_path)).symlink_to(str(symlink_target_path), True)
+
+ app = web.Application()
+
+ # Register global static route:
+ app.router.add_static("/", str(sandbox_path), follow_symlinks=True)
+ client = await aiohttp_client(app)
+
+ await client.start_server()
+ # We need to use a raw socket to test this, as the client will normalize
+ # the path before sending it to the server.
+ reader, writer = await asyncio.open_connection(client.host, client.port)
+ writer.write(b"GET /my_symlink/../private_file HTTP/1.1\r\n\r\n")
+ response = await reader.readuntil(b"\r\n\r\n")
+ assert b"404 Not Found" in response
+ writer.close()
+ await writer.wait_closed()
+
+ reader, writer = await asyncio.open_connection(client.host, client.port)
+ writer.write(b"GET /my_symlink/symlink_target_file HTTP/1.1\r\n\r\n")
+ response = await reader.readuntil(b"\r\n\r\n")
+ assert b"200 OK" in response
+ response = await reader.readuntil(b"readable")
+ assert response == b"readable"
+ writer.close()
+ await writer.wait_closed()
+ await client.close()
+
+
@pytest.mark.parametrize(
"dir_name,filename,data",
[

1
pkgs/development/python-modules/aiohttp/default.nix
View File

@@ -48,6 +48,7 @@ buildPythonPackage rec {
url = "https://github.com/aio-libs/aiohttp/commit/7dcc235cafe0c4521bbbf92f76aecc82fee33e8b.patch";
hash = "sha256-ZzhlE50bmA+e2XX2RH1FuWQHZIAa6Dk/hZjxPoX5t4g=";
})
./3.8.6-CVE-2024-23334.patch
];
postPatch = ''