CVE-2021-42134: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: The Unicorn framework before 0.36.1 for Django allows XSS via a component. NOTE: this issue exists because of an incomplete fix for CVE-2021-42053.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2021-10-11T01:15Z
Repository: https://github.com/adamghill/django-unicorn
Language: Python
Files changed: django_unicorn/components/unicorn_template_response.py, django_unicorn/components/unicorn_view.py, django_unicorn/views/__init__.py, tests/components/test_component.py, tests/components/test_unicorn_template_response.py, tests/templates/test_component_kwargs.html, tests/templates/test_component_kwargs_with_html_entity.html, tests/templates/test_component_variable.html, tests/templatetags/test_unicorn_render.py, tests/views/message/test_sync_input.py, tests/views/test_process_component_request.py
Commit date: 2021-10-08 22:09:35 -0400
Commit message: More complete handling for preventing XSS attacks.

Diff: diff --git a/django_unicorn/components/unicorn_template_response.py b/django_unicorn/components/unicorn_template_response.py
index 09c342d..a6363c9 100644
--- a/django_unicorn/components/unicorn_template_response.py
+++ b/django_unicorn/components/unicorn_template_response.py
@@ -1,12 +1,12 @@
 import logging
 
 from django.template.response import TemplateResponse
-from django.utils.safestring import mark_safe
 
 import orjson
 from bs4 import BeautifulSoup
+from bs4.dammit import EntitySubstitution
 from bs4.element import Tag
 from bs4.formatter import HTMLFormatter
 
 from django_unicorn.utils import sanitize_html
 
@@ -20,10 +20,13 @@ logger = logging.getLogger(__name__)
 class UnsortedAttributes(HTMLFormatter):
     """
     Prevent beautifulsoup from re-ordering attributes.
     """
 
+    def __init__(self):
+        super().__init__(entity_substitution=EntitySubstitution.substitute_html)
+
     def attributes(self, tag: Tag):
         for k, v in tag.attrs.items():
             yield k, v
 
 
@@ -113,11 +116,10 @@ class UnicornTemplateResponse(TemplateResponse):
 
                 for t in json_tags:
                     root_element.insert_after(t)
 
         rendered_template = UnicornTemplateResponse._desoupify(soup)
-        rendered_template = mark_safe(rendered_template)
         self.component.rendered(rendered_template)
 
         response.content = rendered_template
 
         return response
diff --git a/django_unicorn/components/unicorn_view.py b/django_unicorn/components/unicorn_view.py
index 138d959..58e4d27 100644
--- a/django_unicorn/components/unicorn_view.py
+++ b/django_unicorn/components/unicorn_view.py
@@ -8,11 +8,10 @@ from typing import Any, Callable, Dict, List, Optional, Sequence, Type, Union
 from django.conf import settings
 from django.core.cache import caches
 from django.core.exceptions import ImproperlyConfigured
 from django.db.models import Model
 from django.http import HttpRequest
-from django.utils.html import conditional_escape
 from django.views.generic.base import TemplateView
 
 from cachetools.lru import LRUCache
 
 from django_unicorn.settings import get_cache_alias
@@ -339,18 +338,10 @@ class UnicornView(TemplateView):
             if isinstance(self.Meta.javascript_exclude, Sequence):
                 for field_name in self.Meta.javascript_exclude:
                     if field_name in frontend_context_variables:
                         del frontend_context_variables[field_name]
 
-        safe_fields = []
-        # Keep a list of fields that are safe to not sanitize from `frontend_context_variables`
-        if hasattr(self, "Meta") and hasattr(self.Meta, "safe"):
-            if isinstance(self.Meta.safe, Sequence):
-                for field_name in self.Meta.safe:
-                    if field_name in frontend_context_variables:
-                        safe_fields.append(field_name)
-
         # Add cleaned values to `frontend_content_variables` based on the widget in form's fields
         form = self._get_form(attributes)
 
         if form:
             form.is_valid()
@@ -370,22 +361,10 @@ class UnicornView(TemplateView):
                             not hasattr(frontend_context_variables[key], "strip")
                             or frontend_context_variables[key].strip() != value
                         ):
                             frontend_context_variables[key] = value
 
-        for (
-            frontend_context_variable_key,
-            frontend_context_variable_value,
-        ) in frontend_context_variables.items():
-            if (
-                isinstance(frontend_context_variable_value, str)
-                and frontend_context_variable_key not in safe_fields
-            ):
-                frontend_context_variables[frontend_context_variable_key] = escape(
-                    frontend_context_variable_value
-                )
-
         encoded_frontend_context_variables = serializer.dumps(
             frontend_context_variables
         )
 
         return encoded_frontend_context_variables
diff --git a/django_unicorn/views/__init__.py b/django_unicorn/views/__init__.py
index a810a8a..ebaa8b1 100644
--- a/django_unicorn/views/__init__.py
+++ b/django_unicorn/views/__init__.py
@@ -1,13 +1,14 @@
 import copy
 import logging
 from functools import wraps
-from typing import Dict
+from typing import Dict, Sequence
 
 from django.core.cache import caches
 from django.http import HttpRequest, JsonResponse
 from django.http.response import HttpResponseNotModified
+from django.utils.safestring import mark_safe
 from django.views.decorators.csrf import csrf_protect
 from django.views.decorators.http import require_POST
 
 import orjson
 from bs4 import BeautifulSoup
@@ -124,10 +125,24 @@ def _process_component_request(
     component.complete()
 
     # Re-load frontend context variables to deal with non-serializable properties
     component_request.data = orjson.loads(component.get_frontend_context_variables())
 
+    # Get set of attributes that should be marked as `safe`
+    safe_fields = []
+    if hasattr(component, "Meta") and hasattr(component.Meta, "safe"):
+        if isinstance(component.Meta.safe, Sequence):
+            for field_name in component.Meta.safe:
+                if field_name in component._attributes().keys():
+                    safe_fields.append(field_name)
+
+    # Mark safe attributes as such before rendering
+    for field_name in safe_fields:
+        value = getattr(component, field_name)
+        if isinstance(value, str):
+            setattr(component, field_name, mark_safe(value))
+
     # Send back all available data for reset or refresh actions
     updated_data = component_request.data
 
     if not is_reset_called:
         if not is_refresh_called:
diff --git a/tests/components/test_component.py b/tests/components/test_component.py
index f143724..02e1184 100644
--- a/tests/components/test_component.py
+++ b/tests/components/test_component.py
@@ -80,43 +80,10 @@ def test_get_frontend_context_variables(component):
     frontend_context_variables_dict = orjson.loads(frontend_context_variables)
     assert len(frontend_context_variables_dict) == 1
     assert frontend_context_variables_dict.get("name") == "World"
 
 
-def test_get_frontend_context_variables_xss(component):
-    # Set component.name to a potential XSS attack
-    component.name = '<a><style>@keyframes x{}</style><a style="animation-name:x" onanimationend="alert(1)"></a>'
-
-    frontend_context_variables = component.get_frontend_context_variables()
-    frontend_context_variables_dict = orjson.loads(frontend_context_variables)
-    assert len(frontend_context_variables_dict) == 1
-    assert (
-        frontend_context_variables_dict.get("name")
-        == "&lt;a&gt;&lt;style&gt;@keyframes x{}&lt;/style&gt;&lt;a style=&quot;animation-name:x&quot; onanimationend=&quot;alert(1)&quot;&gt;&lt;/a&gt;"
-    )
-
-
-def test_get_frontend_context_variables_safe(component):
-    # Set component.name to a potential XSS attack
-    component.name = '<a><style>@keyframes x{}</style><a style="animation-name:x" onanimationend="alert(1)"></a>'
-
-    class Meta:
-        safe = [
-            "name",
-        ]
-
-    setattr(component, "Meta", Meta())
-
-    frontend_context_variables = component.get_frontend_context_variables()
-    frontend_context_variables_dict = orjson.loads(frontend_context_variables)
-    assert len(frontend_context_variables_dict) == 1
-    assert (
-        frontend_context_variables_dict.get("name")
-        == '<a><style>@keyframes x{}</style><a style="animation-name:x" onanimationend="alert(1)"></a>'
-    )
-
-
 def test_get_context_data(component):
     context_data = component.get_context_data()
     assert (
         len(context_data) == 4
     )  # `unicorn` and `view` are added to context data by default
diff --git a/tests/components/test_unicorn_template_response.py b/tests/components/test_unicorn_template_response.py
index 30b908b..e0f6333 100644
--- a/tests/components/test_unicorn_template_response.py
+++ b/tests/components/test_unicorn_template_response.py
@@ -1,9 +1,12 @@
 import pytest
 from bs4 import BeautifulSoup
 
-from django_unicorn.components.unicorn_template_response import get_root_element
+from django_unicorn.components.unicorn_template_response import (
+    UnicornTemplateResponse,
+    get_root_element,
+)
 
 
 def test_get_root_element():
     expected = "<div>test</div>"
 
@@ -42,5 +45,16 @@ def test_get_root_element_no_element():
 
     with pytest.raises(Exception):
         actual = get_root_element(soup)
 
         assert str(actual) == expected
+
+
+def test_desoupify():
+    html = "<div>&lt;a&gt;&lt;style&gt;@keyframes x{}&lt;/style&gt;&lt;a style=&quot;animation-name:x&quot; onanimationend=&quot;alert(1)&quot;&gt;&lt;/a&gt;!\n</div>\n\n<script type=\"application/javascript\">\n  window.addEventListener('DOMContentLoaded', (event) => {\n    Unicorn.addEventListener('updated', (component) => console.log('got updated', component));\n  });\n</script>"
+    expected = "<div>&lt;a&gt;&lt;style&gt;@keyframes x{}&lt;/style&gt;&lt;a style=\"animation-name:x\" onanimationend=\"alert(1)\"&gt;&lt;/a&gt;!\n</div>\n<script type=\"application/javascript\">\n  window.addEventListener('DOMContentLoaded', (event) => {\n    Unicorn.addEventListener('updated', (component) => console.log('got updated', component));\n  });\n</script>"
+
+    soup = BeautifulSoup(html, "html.parser")
+
+    actual = UnicornTemplateResponse._desoupify(soup)
+
+    assert expected == actual
diff --git a/tests/templates/test_component_kwargs.html b/tests/templates/test_component_kwargs.html
index 66396cf..301274b 100644
--- a/tests/templates/test_component_kwargs.html
+++ b/tests/templates/test_component_kwargs.html
@@ -1,3 +1,3 @@
 <div>
-    ->{{ hello }}<-
+  <b>{{ hello }}</b>
 </div>
\ No newline at end of file
diff --git a/tests/templates/test_component_kwargs_with_html_entity.html b/tests/templates/test_component_kwargs_with_html_entity.html
new file mode 100644
index 0000000..dc4c9a1
--- /dev/null
+++ b/tests/templates/test_component_kwargs_with_html_entity.html
@@ -0,0 +1,3 @@
+<div>
+  ->{{ hello }}<-
+</div>
\ No newline at end of file
diff --git a/tests/templates/test_component_variable.html b/tests/templates/test_component_variable.html
new file mode 100644
index 0000000..adf2bbc
--- /dev/null
+++ b/tests/templates/test_component_variable.html
@@ -0,0 +1,3 @@
+<div>
+  {{ hello }}
+</div>
\ No newline at end of file
diff --git a/tests/templatetags/test_unicorn_render.py b/tests/templatetags/test_unicorn_render.py
index f112cea..96daf71 100644
--- a/tests/templatetags/test_unicorn_render.py
+++ b/tests/templatetags/test_unicorn_render.py
@@ -21,10 +21,19 @@ class FakeComponentKwargs(UnicornView):
     def __init__(self, *args, **kwargs):
         super().__init__(**kwargs)
         self.hello = kwargs.get("test_kwarg")
 
 
+class FakeComponentKwargsWithHtmlEntity(UnicornView):
+    template_name = "templates/test_component_kwargs_with_html_entity.html"
+    hello = "world"
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(**kwargs)
+        self.hello = kwargs.get("test_kwarg")
+
+
 class FakeComponentModel(UnicornView):
     template_name = "templates/test_component_model.html"
     model_id = None
 
     def __init__(self, *args, **kwargs):
@@ -53,11 +62,11 @@ def test_unicorn_render_kwarg():
     )
     unicorn_node = unicorn(None, token)
     context = {}
     actual = unicorn_node.render(context)
 
-    assert "->tested!<-" in actual
+    assert "<b>tested!</b>" in actual
 
 
 def test_unicorn_render_context_variable():
     token = Token(
         TokenType.TEXT,
@@ -65,11 +74,23 @@ def test_unicorn_render_context_variable():
     )
     unicorn_node = unicorn(None, token)
     context = {"test_var": {"nested": "variable!"}}
     actual = unicorn_node.render(context)
 
-    assert "->variable!<-" in actual
+    assert "<b>variable!</b>" in actual
+
+
+def test_unicorn_render_with_invalid_html():
+    token = Token(
+        TokenType.TEXT,
+        "unicorn 'tests.templatetags.test_unicorn_render.FakeComponentKwargsWithHtmlEntity' test_kwarg=test_var.nested",
+    )
+    unicorn_node = unicorn(None, token)
+    context = {"test_var": {"nested": "variable!"}}
+    actual = unicorn_node.render(context)
+
+    assert "-&gt;variable!&lt;-" in actual
 
 
 def test_unicorn_render_parent(settings):
     settings.DEBUG = True
     token = Token(
diff --git a/tests/views/message/test_sync_input.py b/tests/views/message/test_sync_input.py
index d411e95..719f56d 100644
--- a/tests/views/message/test_sync_input.py
+++ b/tests/views/message/test_sync_input.py
@@ -1,7 +1,5 @@
-import orjson
-
 from tests.views.message.utils import post_and_get_response
 
 
 def test_message_nested_sync_input(client):
     data = {"dictionary": {"name": "test"}}
diff --git a/tests/views/test_process_component_request.py b/tests/views/test_process_component_request.py
new file mode 100644
index 0000000..9ab26b2
--- /dev/null
+++ b/tests/views/test_process_component_request.py
@@ -0,0 +1,51 @@
+from django_unicorn.components import UnicornView
+from tests.views.message.utils import post_and_get_response
+
+
+class FakeComponent(UnicornView):
+    template_name = "templates/test_component_variable.html"
+
+    hello = ""
+
+
+class FakeComponentSafe(UnicornView):
+    template_name = "templates/test_component_variable.html"
+
+    hello = ""
+
+    class Meta:
+        safe = ("hello",)
+
+
+def test_html_entities_encoded(client):
+    data = {"hello": "test"}
+    action_queue = [
+        {"payload": {"name": "hello", "value": "<b>test1</b>"}, "type": "syncInput",}
+    ]
+    response = post_and_get_response(
+        client,
+        url="/message/tests.views.test_process_component_request.FakeComponent",
+        data=data,
+        action_queue=action_queue,
+    )
+
+    assert not response["errors"]
+    assert response["data"].get("hello") == "<b>test1</b>"
+    assert "&lt;b&gt;test1&lt;/b&gt;" in response["dom"]
+
+
+def test_safe_html_entities_not_encoded(client):
+    data = {"hello": "test"}
+    action_queue = [
+        {"payload": {"name": "hello", "value": "<b>test1</b>"}, "type": "syncInput",}
+    ]
+    response = post_and_get_response(
+        client,
+        url="/message/tests.views.test_process_component_request.FakeComponentSafe",
+        data=data,
+        action_queue=action_queue,
+    )
+
+    assert not response["errors"]
+    assert response["data"].get("hello") == "<b>test1</b>"
+    assert "<b>test1</b>" in response["dom"]

Vulnerable code: from django.utils.safestring import mark_safe
        rendered_template = mark_safe(rendered_template)
from django.utils.html import conditional_escape
        safe_fields = []
        # Keep a list of fields that are safe to not sanitize from `frontend_context_variables`
        if hasattr(self, "Meta") and hasattr(self.Meta, "safe"):
            if isinstance(self.Meta.safe, Sequence):
                for field_name in self.Meta.safe:
                    if field_name in frontend_context_variables:
                        safe_fields.append(field_name)

        for (
            frontend_context_variable_key,
            frontend_context_variable_value,
        ) in frontend_context_variables.items():
            if (
                isinstance(frontend_context_variable_value, str)
                and frontend_context_variable_key not in safe_fields
            ):
                frontend_context_variables[frontend_context_variable_key] = escape(
                    frontend_context_variable_value
                )

from typing import Dict
def test_get_frontend_context_variables_xss(component):
    # Set component.name to a potential XSS attack
    component.name = '<a><style>@keyframes x{}</style><a style="animation-name:x" onanimationend="alert(1)"></a>'

    frontend_context_variables = component.get_frontend_context_variables()
    frontend_context_variables_dict = orjson.loads(frontend_context_variables)
    assert len(frontend_context_variables_dict) == 1
    assert (
        frontend_context_variables_dict.get("name")
        == "&lt;a&gt;&lt;style&gt;@keyframes x{}&lt;/style&gt;&lt;a style=&quot;animation-name:x&quot; onanimationend=&quot;alert(1)&quot;&gt;&lt;/a&gt;"
    )


def test_get_frontend_context_variables_safe(component):
    # Set component.name to a potential XSS attack
    component.name = '<a><style>@keyframes x{}</style><a style="animation-name:x" onanimationend="alert(1)"></a>'

    class Meta:
        safe = [
            "name",
        ]

    setattr(component, "Meta", Meta())

    frontend_context_variables = component.get_frontend_context_variables()
    frontend_context_variables_dict = orjson.loads(frontend_context_variables)
    assert len(frontend_context_variables_dict) == 1
    assert (
        frontend_context_variables_dict.get("name")
        == '<a><style>@keyframes x{}</style><a style="animation-name:x" onanimationend="alert(1)"></a>'
    )


from django_unicorn.components.unicorn_template_response import get_root_element
    ->{{ hello }}<-
    assert "->tested!<-" in actual
    assert "->variable!<-" in actual
import orjson

Fixed code: from bs4.dammit import EntitySubstitution
    def __init__(self):
        super().__init__(entity_substitution=EntitySubstitution.substitute_html)

from typing import Dict, Sequence
from django.utils.safestring import mark_safe
    # Get set of attributes that should be marked as `safe`
    safe_fields = []
    if hasattr(component, "Meta") and hasattr(component.Meta, "safe"):
        if isinstance(component.Meta.safe, Sequence):
            for field_name in component.Meta.safe:
                if field_name in component._attributes().keys():
                    safe_fields.append(field_name)

    # Mark safe attributes as such before rendering
    for field_name in safe_fields:
        value = getattr(component, field_name)
        if isinstance(value, str):
            setattr(component, field_name, mark_safe(value))

from django_unicorn.components.unicorn_template_response import (
    UnicornTemplateResponse,
    get_root_element,
)


def test_desoupify():
    html = "<div>&lt;a&gt;&lt;style&gt;@keyframes x{}&lt;/style&gt;&lt;a style=&quot;animation-name:x&quot; onanimationend=&quot;alert(1)&quot;&gt;&lt;/a&gt;!\n</div>\n\n<script type=\"application/javascript\">\n  window.addEventListener('DOMContentLoaded', (event) => {\n    Unicorn.addEventListener('updated', (component) => console.log('got updated', component));\n  });\n</script>"
    expected = "<div>&lt;a&gt;&lt;style&gt;@keyframes x{}&lt;/style&gt;&lt;a style=\"animation-name:x\" onanimationend=\"alert(1)\"&gt;&lt;/a&gt;!\n</div>\n<script type=\"application/javascript\">\n  window.addEventListener('DOMContentLoaded', (event) => {\n    Unicorn.addEventListener('updated', (component) => console.log('got updated', component));\n  });\n</script>"

    soup = BeautifulSoup(html, "html.parser")

    actual = UnicornTemplateResponse._desoupify(soup)

    assert expected == actual
  <b>{{ hello }}</b>
<div>
  ->{{ hello }}<-
</div>
<div>
  {{ hello }}
</div>
class FakeComponentKwargsWithHtmlEntity(UnicornView):
    template_name = "templates/test_component_kwargs_with_html_entity.html"
    hello = "world"

    def __init__(self, *args, **kwargs):
        super().__init__(**kwargs)
        self.hello = kwargs.get("test_kwarg")


    assert "<b>tested!</b>" in actual
    assert "<b>variable!</b>" in actual


def test_unicorn_render_with_invalid_html():
    token = Token(
        TokenType.TEXT,
        "unicorn 'tests.templatetags.test_unicorn_render.FakeComponentKwargsWithHtmlEntity' test_kwarg=test_var.nested",
    )
    unicorn_node = unicorn(None, token)
    context = {"test_var": {"nested": "variable!"}}
    actual = unicorn_node.render(context)

    assert "-&gt;variable!&lt;-" in actual
from django_unicorn.components import UnicornView
from tests.views.message.utils import post_and_get_response


class FakeComponent(UnicornView):
    template_name = "templates/test_component_variable.html"

    hello = ""


class FakeComponentSafe(UnicornView):
    template_name = "templates/test_component_variable.html"

    hello = ""

    class Meta:
        safe = ("hello",)


def test_html_entities_encoded(client):
    data = {"hello": "test"}
    action_queue = [
        {"payload": {"name": "hello", "value": "<b>test1</b>"}, "type": "syncInput",}
    ]
    response = post_and_get_response(
        client,
        url="/message/tests.views.test_process_component_request.FakeComponent",
        data=data,
        action_queue=action_queue,
    )

    assert not response["errors"]
    assert response["data"].get("hello") == "<b>test1</b>"
    assert "&lt;b&gt;test1&lt;/b&gt;" in response["dom"]


def test_safe_html_entities_not_encoded(client):
    data = {"hello": "test"}
    action_queue = [
        {"payload": {"name": "hello", "value": "<b>test1</b>"}, "type": "syncInput",}
    ]
    response = post_and_get_response(
        client,
        url="/message/tests.views.test_process_component_request.FakeComponentSafe",
        data=data,
        action_queue=action_queue,
    )

    assert not response["errors"]
    assert response["data"].get("hello") == "<b>test1</b>"
    assert "<b>test1</b>" in response["dom"]