Repository: django/channels
Pull request #1575 (2020-11-15 13:31:48)

bug in scope refered from self
bug in scope variable refers in local variable and not in class variable so bug will raise attribute error "scope attribute not in self"


Patch:
diff --git a/CHANGELOG.txt b/CHANGELOG.txt
index a14c514..b8f4fb7 100644
--- a/CHANGELOG.txt
+++ b/CHANGELOG.txt
@@ -1,11 +1,22 @@
 Full release notes, with more details and upgrade information, are available at:
 https://channels.readthedocs.io/en/latest/releases
 
+3.0.3 (2020-12-28)
+------------------
+
+* Fixed a bug in Channels 3.0 where the legacy ``channels.http.AsgiHandler``
+  would not correctly isolate per-request scopes.
+
+  This is a security release for CVE-2020-35681. Please see the `Version 3.0.3
+  release notes
+  <https://channels.readthedocs.io/en/latest/releases/3.0.3.html>`_ for full
+  details.
+
 
 3.0.2 (2020-11-9)
 -----------------
 
-* Fixes a bug in Channels 3.0 where `StaticFilesWrapper` was not updated to
+* Fixes a bug in Channels 3.0 where ``StaticFilesWrapper`` was not updated to
   the ASGI 3 single-callable interface.
 
 * Users of the ``runworker`` command should ensure to update ``asgiref`` to
diff --git a/channels/__init__.py b/channels/__init__.py
index 84cee51..47cfcee 100644
--- a/channels/__init__.py
+++ b/channels/__init__.py
@@ -1,4 +1,4 @@
-__version__ = "3.0.2"
+__version__ = "3.0.3"
 
 try:
     import django
diff --git a/channels/http.py b/channels/http.py
index c55bf9b..d56064e 100644
--- a/channels/http.py
+++ b/channels/http.py
@@ -171,7 +171,7 @@ class AsgiHandler(base.BaseHandler):
 
     def __init__(self):
         msg = (
-            "Channel's inbuilt http protocal AsgiHandler is deprecated. "
+            "Channel's inbuilt http protocol AsgiHandler is deprecated. "
             "Use Django's get_asgi_application() instead."
         )
         warnings.warn(msg, DeprecationWarning, stacklevel=2)
@@ -188,8 +188,6 @@ class AsgiHandler(base.BaseHandler):
                 "The AsgiHandler can only handle HTTP connections, not %s"
                 % scope["type"]
             )
-        self.scope = scope
-        self.send = async_to_sync(send)
 
         # Receive the HTTP request body as a stream object.
         try:
@@ -197,7 +195,7 @@ class AsgiHandler(base.BaseHandler):
         except RequestAborted:
             return
         # Launch into body handling (and a synchronous subthread).
-        await self.handle(body_stream)
+        await self.handle(scope, async_to_sync(send), body_stream)
 
     async def read_body(self, receive):
         """Reads a HTTP body from an ASGI connection."""
@@ -220,19 +218,19 @@ class AsgiHandler(base.BaseHandler):
         return body_file
 
     @sync_to_async
-    def handle(self, body):
+    def handle(self, scope, send, body):
         """
         Synchronous message processing.
         """
         # Set script prefix from message root_path, turning None into empty string
-        script_prefix = self.scope.get("root_path", "") or ""
+        script_prefix = scope.get("root_path", "") or ""
         if settings.FORCE_SCRIPT_NAME:
             script_prefix = settings.FORCE_SCRIPT_NAME
         set_script_prefix(script_prefix)
-        signals.request_started.send(sender=self.__class__, scope=self.scope)
+        signals.request_started.send(sender=self.__class__, scope=scope)
         # Run request through view system
         try:
-            request = self.request_class(self.scope, body)
+            request = self.request_class(scope, body)
         except UnicodeDecodeError:
             logger.warning(
                 "Bad Request (UnicodeDecodeError)",
@@ -255,7 +253,7 @@ class AsgiHandler(base.BaseHandler):
                 response.block_size = 1024 * 512
         # Transform response into messages, which we yield back to caller
         for response_message in self.encode_response(response):
-            self.send(response_message)
+            send(response_message)
         # Close the response now we're done with it
         response.close()
 
diff --git a/channels/staticfiles.py b/channels/staticfiles.py
index 73c7dfb..f625976 100644
--- a/channels/staticfiles.py
+++ b/channels/staticfiles.py
@@ -51,11 +51,15 @@ class StaticFilesHandler(AsgiHandler):
 
     # TODO: Review hierarchy here. Do we NEED to inherit BaseHandler, AsgiHandler?
 
+    async def __call__(self, scope, receive, send):
+        self.static_base_url = scope["static_base_url"][2]
+        return await super().__call__(scope, receive, send)
+
     def file_path(self, url):
         """
         Returns the relative path to the media file on disk for the given URL.
         """
-        relative_url = url[len(self.scope["static_base_url"][2]) :]
+        relative_url = url[len(self.static_base_url) :]
         return url2pathname(relative_url)
 
     def serve(self, request):
diff --git a/docs/releases/3.0.3.rst b/docs/releases/3.0.3.rst
new file mode 100644
index 0000000..1f1e041
--- /dev/null
+++ b/docs/releases/3.0.3.rst
@@ -0,0 +1,54 @@
+3.0.3 Release Notes
+===================
+
+Channels 3.0.3 fixes a security issue in Channels 3.0.2
+
+CVE-2020-35681: Potential leakage of session identifiers using legacy ``AsgiHandler``
+-------------------------------------------------------------------------------------
+
+The legacy ``channels.http.AsgiHandler`` class, used for handling HTTP type
+requests in an ASGI environment prior to Django 3.0, did not correctly separate
+request scopes in Channels 3.0. In many cases this would result in a crash but,
+with correct timing responses could be sent to the wrong client, resulting in
+potential leakage of session identifiers and other sensitive data.
+
+This issue affects Channels 3.0.x before 3.0.3, and is resolved in Channels
+3.0.3.
+
+Users of ``ProtocolTypeRouter`` not explicitly specifying the handler for the
+``'http'`` key, or those explicitly using ``channels.http.AsgiHandler``, likely
+to support Django v2.2, are affected and should update immediately.
+
+Note that both an unspecified handler for the ``'http'`` key and using
+``channels.http.AsgiHandler`` are deprecated, and will raise a warning, from
+Channels v3.0.0
+
+This issue affects only the legacy channels provided class, and not Django's
+similar ``ASGIHandler``, available from Django 3.0. It is recommended to update
+to Django 3.0+ and use the Django provided ``ASGIHandler``.
+
+A simplified ``asgi.py`` script will look like this:
+
+.. code-block:: python
+
+    import os
+
+    from django.conf.urls import url
+    from django.core.asgi import get_asgi_application
+
+    # Fetch Django ASGI application early to ensure AppRegistry is populated
+    # before importing consumers and AuthMiddlewareStack that may import ORM
+    # models.
+    os.environ.setdefault("DJANGO_SETTINGS_MODULE", "mysite.settings")
+    django_asgi_app = get_asgi_application()
+
+    # Import other Channels classes and consumers here.
+    from channels.routing import ProtocolTypeRouter, URLRouter
+
+    application = ProtocolTypeRouter({
+        # Explicitly set 'http' key using Django's ASGI application.
+        "http": django_asgi_app,
+        ),
+    })
+
+Please see  :doc:`/deploying` for a more complete example.
diff --git a/docs/releases/index.rst b/docs/releases/index.rst
index 1f42430..343e5e5 100644
--- a/docs/releases/index.rst
+++ b/docs/releases/index.rst
@@ -4,31 +4,32 @@ Release Notes
 .. toctree::
    :maxdepth: 1
 
-   1.0.0
-   1.0.1
-   1.0.2
-   1.0.3
-   1.1.0
-   1.1.1
-   1.1.2
-   1.1.3
-   1.1.4
-   1.1.5
-   1.1.6
-   2.0.0
-   2.0.1
-   2.0.2
-   2.1.0
-   2.1.1
-   2.1.2
-   2.1.3
-   2.1.4
-   2.1.5
-   2.1.6
-   2.1.7
-   2.2.0
-   2.3.0
-   2.4.0
-   3.0.0
-   3.0.1
+   3.0.3
    3.0.2
+   3.0.1
+   3.0.0
+   2.4.0
+   2.3.0
+   2.2.0
+   2.1.7
+   2.1.6
+   2.1.5
+   2.1.4
+   2.1.3
+   2.1.2
+   2.1.1
+   2.1.0
+   2.0.2
+   2.0.1
+   2.0.0
+   1.1.6
+   1.1.5
+   1.1.4
+   1.1.3
+   1.1.2
+   1.1.1
+   1.1.0
+   1.0.3
+   1.0.2
+   1.0.1
+   1.0.0
diff --git a/docs/topics/authentication.rst b/docs/topics/authentication.rst
index fa26661..5587cc4 100644
--- a/docs/topics/authentication.rst
+++ b/docs/topics/authentication.rst
@@ -96,7 +96,7 @@ query string and uses that:
             # Look up user from query string (you should also do things like
             # checking if it is a valid user ID, or if scope["user"] is already
             # populated).
-            scope['user'] = await get_user(int(self.scope["query_string"]))
+            scope['user'] = await get_user(int(scope["query_string"]))
 
             return await self.app(scope, receive, send)
 
diff --git a/docs/topics/consumers.rst b/docs/topics/consumers.rst
index 5159383..a502798 100644
--- a/docs/topics/consumers.rst
+++ b/docs/topics/consumers.rst
@@ -112,7 +112,7 @@ callable into an asynchronous coroutine.
 .. important::
 
     If you want to call the Django ORM from an ``AsyncConsumer`` (or any other
-    synchronous code), you should use the ``database_sync_to_async`` adapter
+    asynchronous code), you should use the ``database_sync_to_async`` adapter
     instead. See :doc:`/topics/databases` for more.
 
 
diff --git a/docs/topics/worker.rst b/docs/topics/worker.rst
index 1500016..f9699d9 100644
--- a/docs/topics/worker.rst
+++ b/docs/topics/worker.rst
@@ -15,6 +15,8 @@ names, as a simple, very-low-latency task queue.
     We recommend you use it for work that does not need guarantees around
     being complete (at-most-once delivery), and for work that needs more
     guarantees, look into a separate dedicated task queue.
+    
+    This feature does not work with the in-memory channel layer.
 
 Setting up background tasks works in two parts - sending the events, and then
 setting up the consumers to receive and process the events.
@@ -58,8 +60,8 @@ name. We recommend you use ProtocolTypeRouter and ChannelNameRouter (see
     application = ProtocolTypeRouter({
         ...
         "channel": ChannelNameRouter({
-            "thumbnails-generate": consumers.GenerateConsumer,
-            "thumbnails-delete": consumers.DeleteConsumer,
+            "thumbnails-generate": consumers.GenerateConsumer.as_asgi(),
+            "thumbnails-delete": consumers.DeleteConsumer.as_asgi(),
         }),
     })
 


Test patch:
diff --git a/tests/test_http.py b/tests/test_http.py
index d4feae2..68ec311 100644
--- a/tests/test_http.py
+++ b/tests/test_http.py
@@ -351,6 +351,35 @@ async def test_handler_body_ignore_extra():
     assert body_stream.read() == b"chunk one"
 
 
+@pytest.mark.django_db
+@pytest.mark.asyncio
+@pytest.mark.filterwarnings("ignore::DeprecationWarning")
+async def test_handler_concurrent_requests():
+    """
+    Tests request handling ignores anything after more_body: False
+    """
+    scope = {"type": "http", "http_version": "1.1", "method": "GET", "path": "/test/"}
+    handler = MockHandler()
+    comm_1 = ApplicationCommunicator(handler, {**scope})
+    comm_2 = ApplicationCommunicator(handler, {**scope})
+
+    request_1 = comm_1.send_input(
+        {"type": "http.request", "body": b"request 1", "more_body": False}
+    )
+    request_2 = comm_2.send_input(
+        {"type": "http.request", "body": b"request 2", "more_body": False}
+    )
+
+    await request_1
+    await request_2
+
+    await comm_1.receive_output(1)  # response start
+    await comm_1.receive_output(1)  # response body
+
+    await comm_2.receive_output(1)  # response start
+    await comm_2.receive_output(1)  # response body
+
+
 @pytest.mark.django_db(transaction=True)
 @pytest.mark.asyncio
 async def test_sessions():
diff --git a/tests/test_staticfiles.py b/tests/test_staticfiles.py
index 61afea4..126d39b 100644
--- a/tests/test_staticfiles.py
+++ b/tests/test_staticfiles.py
@@ -1,6 +1,6 @@
 import pytest
 
-from channels.staticfiles import StaticFilesWrapper
+from channels.staticfiles import StaticFilesHandler, StaticFilesWrapper
 
 
 @pytest.fixture(autouse=True)
@@ -99,3 +99,27 @@ def test_is_single_callable():
         "StaticFilesWrapper should be recognized as a single callable by "
         "asgiref compatibility tools"
     )
+
+
+@pytest.mark.asyncio
+async def test_staticfiles_handler_can_generate_file_path():
+    """
+    StaticFilesHandler.file_path must not rely on scope being assigned to self.
+    """
+
+    class MockedHandler(StaticFilesHandler):
+        async def __call__(self, scope, receive, send):
+            # Equivalent setUp from real __call__.
+            request = self.request_class(scope, "")
+            self.static_base_url = scope["static_base_url"][2]
+            # Method under test.
+            return self.file_path(request.path)
+
+    wrapper = StaticFilesWrapper(
+        MockApplication("application"), staticfiles_handler=MockedHandler
+    )
+    scope = request_for_path("/static/image.png")
+    scope["method"] = "GET"
+    assert (
+        await wrapper(scope, None, None) == "/image.png"
+    ), "StaticFilesWrapper should serve paths under the STATIC_URL path"