fix(security): fix cookies leakage between different users

- fix security vulnerabilities of cookies leakage between different users

Security-advisories: GHSA-7vwr-g6pm-9hc8

Author: WSH032

src/fastapi_proxy_lib/core/_tool.py

@@ -19,6 +19,12 @@
 import httpx
 from starlette import status
 from starlette.background import BackgroundTask as BackgroundTask_t
+from starlette.datastructures import (
+    Headers as StarletteHeaders,
+)
+from starlette.datastructures import (
+    MutableHeaders as StarletteMutableHeaders,
+)
 from starlette.responses import JSONResponse
 from starlette.types import Scope
 from typing_extensions import deprecated, overload
@@ -424,3 +430,37 @@ def warn_for_none_filter(
         return default_proxy_filter
     else:
         return proxy_filter
+
+
+def change_necessary_client_header_for_httpx(
+    *, headers: StarletteHeaders, target_url: httpx.URL
+) -> StarletteMutableHeaders:
+    """Change client request headers for sending to proxy server.
+
+    - Change "host" header to `target_url.netloc.decode("ascii")`.
+    - If "Cookie" header is not in headers,
+        will forcibly add a empty "Cookie" header
+        to avoid httpx.AsyncClient automatically add another user cookiejar.
+
+    Args:
+        headers: original client request headers.
+        target_url: httpx.URL of target server url.
+
+    Returns:
+        New requests headers, the copy of original input headers.
+    """
+    # https://www.starlette.io/requests/#headers
+    new_headers = headers.mutablecopy()
+
+    # 将host字段更新为目标url的host
+    # TODO: 如果查看httpx.URL源码，就会发现netloc是被字符串编码成bytes的，能否想个办法直接获取字符串来提高性能?
+    new_headers["host"] = target_url.netloc.decode("ascii")
+
+    # https://developer.mozilla.org/zh-CN/docs/Web/HTTP/Headers/Cookie
+
+    # FIX: https://github.com/WSH032/fastapi-proxy-lib/security/advisories/GHSA-7vwr-g6pm-9hc8
+    # forcibly set `Cookie` header to avoid httpx.AsyncClient automatically add another user cookiejar
+    if "Cookie" not in new_headers:  # case-insensitive
+        new_headers["Cookie"] = ""
+
+    return new_headers

src/fastapi_proxy_lib/core/http.py

@@ -29,6 +29,7 @@
 from ._tool import (
     ProxyFilterProto,
     _RejectedProxyRequestError,  # pyright: ignore [reportPrivateUsage]  # 允许使用本项目内部的私有成员
+    change_necessary_client_header_for_httpx,
     check_base_url,
     check_http_version,
     return_err_msg_response,
@@ -121,9 +122,12 @@ def _change_client_header(
 ) -> _ConnectionHeaderParseResult:
     """Change client request headers for sending to proxy server.
 
+    - Change "host" header to `target_url.netloc.decode("ascii")`.
+    - If "Cookie" header is not in headers,
+        will forcibly add a empty "Cookie" header
+        to avoid httpx.AsyncClient automatically add another user cookiejar.
     - Will remove "close" value in "connection" header, and add "keep-alive" value to it.
     - And remove "keep-alive" header.
-    - And change "host" header to `target_url.netloc.decode("ascii")`.
 
     Args:
         headers: original client request headers.
@@ -135,9 +139,12 @@ def _change_client_header(
             new_headers: New requests headers, the **copy** of original input headers.
     """
     # https://www.starlette.io/requests/#headers
-    # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Connection#syntax
-    new_headers = headers.mutablecopy()
 
+    new_headers = change_necessary_client_header_for_httpx(
+        headers=headers, target_url=target_url
+    )
+
+    # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Connection#syntax
     # NOTE: http标准中规定，connecttion头字段的值用于指示逐段头，而标头是大小写不敏感的，故认为可以转为小写处理
     client_connection_header = [
         v.strip() for v in new_headers.get("connection", "").lower().split(",")
@@ -160,10 +167,6 @@ def _change_client_header(
     if "keep-alive" in new_headers:
         del new_headers["keep-alive"]
 
-    # 将host字段更新为目标url的host
-    # TODO: 如果查看httpx.URL源码，就会发现netloc是被字符串编码成bytes的，能否想个办法直接获取字符串来提高性能?
-    new_headers["host"] = target_url.netloc.decode("ascii")
-
     return _ConnectionHeaderParseResult(whether_require_close, new_headers)
 
 
@@ -251,6 +254,12 @@ async def send_request_to_target(  # pyright: ignore [reportIncompatibleMethodOv
             None if request.method in _NON_REQUEST_BODY_METHODS else request.stream()
         )
 
+        # FIX: https://github.com/WSH032/fastapi-proxy-lib/security/advisories/GHSA-7vwr-g6pm-9hc8
+        # time cost: 396 ns ± 3.39 ns
+        # 由于这不是原子性的操作，所以不保证一定阻止cookie泄漏
+        # 一定能保证修复的方法是通过`_tool.change_necessary_client_header_for_httpx`强制指定优先级最高的cookie头
+        client.cookies.clear()
+
         # NOTE: 不要在这里catch `client.build_request` 和 `client.send` 的异常，因为通常来说
         # - 反向代理的异常需要报 5xx 错误
         # - 而正向代理的异常需要报 4xx 错误

src/fastapi_proxy_lib/core/websocket.py

@@ -24,12 +24,6 @@
     DEFAULT_QUEUE_SIZE,
 )
 from starlette import status as starlette_status
-from starlette.datastructures import (
-    Headers as StarletteHeaders,
-)
-from starlette.datastructures import (
-    MutableHeaders as StarletteMutableHeaders,
-)
 from starlette.exceptions import WebSocketException as StarletteWebSocketException
 from starlette.responses import Response as StarletteResponse
 from starlette.responses import StreamingResponse
@@ -40,6 +34,7 @@
 
 from ._model import BaseProxyModel
 from ._tool import (
+    change_necessary_client_header_for_httpx,
     check_base_url,
     check_http_version,
 )
@@ -82,29 +77,7 @@ class _ClientServerProxyTask(NamedTuple):
 #################### Tools function ####################
 
 
-def _change_client_header(
-    *, headers: StarletteHeaders, target_url: httpx.URL
-) -> StarletteMutableHeaders:
-    """Change client request headers for sending to proxy server.
-
-    - Change "host" header to `target_url.netloc.decode("ascii")`.
-
-    Args:
-        headers: original client request headers.
-        target_url: httpx.URL of target server url.
-
-    Returns:
-        New requests headers, the copy of original input headers.
-    """
-    # https://www.starlette.io/requests/#headers
-    # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Connection#syntax
-    new_headers = headers.mutablecopy()
-
-    # 将host字段更新为目标url的host
-    # TODO: 如果查看httpx.URL源码，就会发现netloc是被字符串编码成bytes的，能否想个办法直接获取字符串来提高性能?
-    new_headers["host"] = target_url.netloc.decode("ascii")
-
-    return new_headers
+_change_client_header = change_necessary_client_header_for_httpx
 
 
 def _get_client_request_subprotocols(ws_scope: Scope) -> Union[List[str], None]:
@@ -540,6 +513,12 @@ async def send_request_to_target(  # pyright: ignore [reportIncompatibleMethodOv
         # https://docs.python.org/3.12/library/contextlib.html?highlight=asyncexitstack#catching-exceptions-from-enter-methods
         stack = AsyncExitStack()
         try:
+            # FIX: https://github.com/WSH032/fastapi-proxy-lib/security/advisories/GHSA-7vwr-g6pm-9hc8
+            # time cost: 396 ns ± 3.39 ns
+            # 由于这不是原子性的操作，所以不保证一定阻止cookie泄漏
+            # 一定能保证修复的方法是通过`_tool.change_necessary_client_header_for_httpx`强制指定优先级最高的cookie头
+            client.cookies.clear()
+
             proxy_ws = await stack.enter_async_context(
                 httpx_ws.aconnect_ws(
                     # 这个是httpx_ws类型注解的问题，其实是可以使用httpx.URL的

tests/app/echo_http_app.py

@@ -12,7 +12,7 @@
 DEFAULT_FILE_NAME = "echo.txt"
 
 
-def get_app() -> AppDataclass4Test:
+def get_app() -> AppDataclass4Test:  # noqa: C901
     """Get the echo http app.
 
     Returns:
@@ -83,6 +83,28 @@ async def echo_headers_and_params(
         }
         return msg
 
+    @app.get("/get/cookies")
+    async def cookies(
+        request: Request,
+    ) -> Mapping[str, str]:
+        """Returns cookie of request."""
+        nonlocal test_app_dataclass
+        test_app_dataclass.request_dict["request"] = request
+        return request.cookies
+
+    @app.get("/get/cookies/set/{key}/{value}")
+    async def cookies_set(
+        request: Request,
+        key: str,
+        value: str,
+    ) -> Response:
+        """Returns a response which requires client to set cookie: `key=value`."""
+        nonlocal test_app_dataclass
+        test_app_dataclass.request_dict["request"] = request
+        response = Response()
+        response.set_cookie(key=key, value=value)
+        return response
+
     @app.post("/post/echo_body")
     async def echo_body(request: Request) -> Response:
         """Http post method endpoint for echo body.

tests/test_http.py

@@ -205,6 +205,41 @@ async def test_bad_url_request(
         assert r.status_code == 502
         check_if_err_resp_is_from_px_serv(r)
 
+    @pytest.mark.anyio()
+    async def test_cookie_leakage(
+        self,
+        tool_4_test_fixture: Tool4TestFixture,
+    ) -> None:
+        """Testing for fixing cookie leakage vulnerabilities."""
+        client_for_conn_to_proxy_server = (
+            tool_4_test_fixture.client_for_conn_to_proxy_server
+        )
+        proxy_server_base_url = tool_4_test_fixture.proxy_server_base_url
+
+        # request to set cookie: foo=bar
+        await client_for_conn_to_proxy_server.get(
+            proxy_server_base_url + "get/cookies/set/foo/bar"
+        )
+        # check if cookie is set
+        assert client_for_conn_to_proxy_server.cookies["foo"] == "bar"
+        r = await client_for_conn_to_proxy_server.get(
+            proxy_server_base_url + "get/cookies"
+        )
+        assert r.json()["foo"] == "bar"
+
+        # Then simulate the access of another user's client by clearing cookiejar
+        client_for_conn_to_proxy_server.cookies.clear()
+        # check if cookiejar is cleared
+        assert not client_for_conn_to_proxy_server.cookies
+
+        # check if cookie is not leaked
+        r = await client_for_conn_to_proxy_server.get(
+            proxy_server_base_url + "get/cookies",
+            cookies={"a": "b"},
+        )
+        assert "foo" not in r.json()  # not leaked
+        assert r.json()["a"] == "b"  # send cookies normally
+
 
 class TestForwardHttpProxy(AbstractTestProxy):
     """For testing forward http proxy."""