Merge pull request #7327 from akatsoulas/update-parser-settings

akatsoulas · web-flow · commit cc798e32684b · 2026-03-19T18:34:09.000+02:00
Restrict parser usage and ID handling in JS
diff --git a/kitsune/dashboards/jinja2/dashboards/includes/macros.html b/kitsune/dashboards/jinja2/dashboards/includes/macros.html
@@ -139,7 +139,7 @@
       {{ pgettext('l10n dashboard', 'Subscribe...') }}
     </button>
 
-    <div id="subscribe-modal" class="mzp-u-modal-content">
+    <div data-modal-id="subscribe-modal" class="mzp-u-modal-content">
       <form action="" method="POST">
         {% csrf_token %}
         <table>
diff --git a/kitsune/forums/models.py b/kitsune/forums/models.py
@@ -9,6 +9,7 @@
 from kitsune.access.utils import has_perm
 from kitsune.flagit.models import FlaggedObject
 from kitsune.sumo.models import ModelBase
+from kitsune.sumo.parser import BASE_ALLOWED_ATTRIBUTES
 from kitsune.sumo.templatetags.jinja_helpers import urlparams, wiki_to_html
 from kitsune.sumo.urlresolvers import reverse
 from kitsune.tidings.models import NotificationsMixin
@@ -252,4 +253,4 @@ def get_absolute_url(self):
 
     @property
     def content_parsed(self):
-        return wiki_to_html(self.content)
+        return wiki_to_html(self.content, attributes=BASE_ALLOWED_ATTRIBUTES)
diff --git a/kitsune/kbforums/models.py b/kitsune/kbforums/models.py
@@ -5,6 +5,7 @@
 
 from kitsune import kbforums
 from kitsune.sumo.models import ModelBase
+from kitsune.sumo.parser import BASE_ALLOWED_ATTRIBUTES
 from kitsune.sumo.templatetags.jinja_helpers import urlparams, wiki_to_html
 from kitsune.sumo.urlresolvers import reverse
 from kitsune.tidings.models import NotificationsMixin
@@ -153,4 +154,4 @@ def get_absolute_url(self):
 
     @property
     def content_parsed(self):
-        return wiki_to_html(self.content)
+        return wiki_to_html(self.content, attributes=BASE_ALLOWED_ATTRIBUTES)
diff --git a/kitsune/questions/jinja2/questions/includes/answer.html b/kitsune/questions/jinja2/questions/includes/answer.html
@@ -97,7 +97,7 @@ <h3 class="is-spam">{{ _('Marked as spam') }}</h3>
     <div class="content">
       {% if is_preview %}
         {# Don't use cached content_parsed #}
-        {{ answer.content|wiki_to_html(locale=request.LANGUAGE_CODE) }}
+        {{ answer.content|wiki_to_html_questions(locale=request.LANGUAGE_CODE) }}
       {% else %}
         {{ answer.content_parsed|safe }}
       {% endif %}
diff --git a/kitsune/questions/jinja2/questions/includes/email_subscribe.html b/kitsune/questions/jinja2/questions/includes/email_subscribe.html
@@ -1,7 +1,7 @@
 {% from "layout/errorlist.html" import errorlist %}
 {% set title = _("Get email updates") %}
 
-<section id="email-subscribe" class="mzp-u-modal-content" title="{{ title }}">
+<section data-modal-id="email-subscribe" class="mzp-u-modal-content" title="{{ title }}">
   <h2 class="sumo-page-subheading">{{ title }}</h2>
   <form action="{{ url('questions.watch', question.id) }}" method="post">
     {% csrf_token %}
diff --git a/kitsune/questions/jinja2/questions/question_details.html b/kitsune/questions/jinja2/questions/question_details.html
@@ -516,7 +516,7 @@ <h3 class="sidebar-subheading sidebar-nav--heading-item is-accordion-heading">{{
                 {% endblock %}
                 <a id="show-more-details" href="javascript:;" data-sumo-modal="more-system-details">{{ _('More system details') }}</a>
 
-                <section id="more-system-details" class="mzp-u-modal-content text-body-md" title="{{ _('Additional System Details') }}" data-target="#show-more-details">
+                <section data-modal-id="more-system-details" class="mzp-u-modal-content text-body-md" title="{{ _('Additional System Details') }}" data-target="#show-more-details">
                   <h2 class="sumo-page-subheading">{{ _('Additional System Details') }}</h2>
                   {% if question.metadata.crash_id %}
                   <h3 class="sumo-card-heading">{{ crash_id_label }}</h3>
diff --git a/kitsune/questions/models.py b/kitsune/questions/models.py
@@ -30,6 +30,7 @@
 from kitsune.questions.managers import AAQConfigManager, AnswerManager, QuestionManager
 from kitsune.sumo.i18n import split_into_language_and_path
 from kitsune.sumo.models import LocaleField, ModelBase
+from kitsune.sumo.parser import BASE_ALLOWED_ATTRIBUTES
 from kitsune.sumo.templatetags.jinja_helpers import urlparams, wiki_to_html
 from kitsune.sumo.urlresolvers import reverse
 from kitsune.sumo.utils import chunked
@@ -1167,7 +1168,7 @@ def _content_parsed(obj, locale):
     cache_key = obj.html_cache_key % obj.id
     html = cache.get(cache_key)
     if html is None:
-        html = wiki_to_html(obj.content, locale)
+        html = wiki_to_html(obj.content, locale, attributes=BASE_ALLOWED_ATTRIBUTES)
         cache.add(cache_key, html, settings.CACHE_MEDIUM_TIMEOUT)
     return html
 
diff --git a/kitsune/sumo/parser.py b/kitsune/sumo/parser.py
@@ -14,15 +14,9 @@
 from kitsune.sumo.sanitize import clean, linkify
 from kitsune.sumo.urlresolvers import reverse
 
-ALLOWED_ATTRIBUTES = {
+BASE_ALLOWED_ATTRIBUTES = {
     "a": ["href", "title", "class", "rel", "data-mozilla-ui-reset", "data-mozilla-ui-preferences"],
-    "div": ["id", "class", "style", "data-for", "title", "data-target", "data-modal"],
-    "h1": ["id"],
-    "h2": ["id"],
-    "h3": ["id"],
-    "h4": ["id"],
-    "h5": ["id"],
-    "h6": ["id"],
+    "div": ["class", "style", "data-for", "title"],
     "li": ["class"],
     "span": ["class", "data-for"],
     "img": ["class", "src", "data-original-src", "alt", "title", "height", "width", "style"],
@@ -37,6 +31,17 @@
     ],
     "source": ["src", "type"],
 }
+
+ALLOWED_ATTRIBUTES = {
+    **BASE_ALLOWED_ATTRIBUTES,
+    "div": BASE_ALLOWED_ATTRIBUTES["div"] + ["id", "data-target", "data-modal"],
+    "h1": ["id"],
+    "h2": ["id"],
+    "h3": ["id"],
+    "h4": ["id"],
+    "h5": ["id"],
+    "h6": ["id"],
+}
 ALLOWED_STYLES = ["vertical-align"]
 IMAGE_PARAMS = ["alt", "align", "caption", "valign", "frame", "page", "link", "width", "height"]
 IMAGE_PARAM_VALUES = {
diff --git a/kitsune/sumo/static/sumo/js/protocol-modal-init.js b/kitsune/sumo/static/sumo/js/protocol-modal-init.js
@@ -7,11 +7,11 @@ import Modal from "protocol/js/modal";
   if (modalLink) {
     modalLink.forEach(function (e) {
       var dialogLink = e.dataset.sumoModal;
-      var content = document.getElementById(dialogLink);
+      var content = document.querySelector('[data-modal-id="' + CSS.escape(dialogLink) + '"]');
       function openThisDialog(e) {
         Modal.createModal(e.target, content, {
           closeText: 'Close modal',
-          content: document.getElementById(e.target.dataset.sumoModal),
+          content: document.querySelector('[data-modal-id="' + CSS.escape(e.target.dataset.sumoModal) + '"]'),
         });
         e.preventDefault();
       };
diff --git a/kitsune/sumo/static/sumo/js/reportabuse.js b/kitsune/sumo/static/sumo/js/reportabuse.js
@@ -8,7 +8,7 @@
   $(function () {
     $('[data-sumo-modal]').each(function () {
       var identifier = $(this).data('sumo-modal');
-      $('#' + identifier + ' [type="submit"]').on('click', function (ev) {
+      $('[data-modal-id="' + identifier + '"] [type="submit"]').on('click', function (ev) {
         ev.preventDefault();
         var $this = $(this);
         var $form = $this.closest('form');
diff --git a/kitsune/sumo/templatetags/jinja_helpers.py b/kitsune/sumo/templatetags/jinja_helpers.py
@@ -26,6 +26,7 @@
 from markupsafe import Markup, escape
 
 from kitsune.sumo import parser, sanitize
+from kitsune.sumo.parser import BASE_ALLOWED_ATTRIBUTES
 from kitsune.sumo.sanitize import ALLOWED_BIO_ATTRIBUTES, ALLOWED_BIO_TAGS, clean
 from kitsune.sumo.urlresolvers import reverse
 from kitsune.sumo.utils import (
@@ -135,6 +136,12 @@ def wiki_to_html(
     )
 
 
+@library.filter
+def wiki_to_html_questions(wiki_markup, locale=settings.WIKI_DEFAULT_LANGUAGE):
+    """Wiki Markup -> HTML using the restricted questions allowlist."""
+    return wiki_to_html(wiki_markup, locale=locale, attributes=BASE_ALLOWED_ATTRIBUTES)
+
+
 @library.filter
 def wiki_to_safe_html(wiki_markup, locale=settings.WIKI_DEFAULT_LANGUAGE, nofollow=True):
     """Wiki Markup -> HTML Markup object with limited tags"""
diff --git a/kitsune/sumo/tests/test_parser.py b/kitsune/sumo/tests/test_parser.py
@@ -6,12 +6,15 @@
 
 from kitsune.gallery.tests import ImageFactory
 from kitsune.sumo.parser import (
+    ALLOWED_ATTRIBUTES,
+    BASE_ALLOWED_ATTRIBUTES,
     IMAGE_PARAM_VALUES,
     IMAGE_PARAMS,
     WikiParser,
     _get_wiki_link,
     build_hook_params,
     get_object_fallback,
+    wiki_to_html,
 )
 from kitsune.sumo.tests import TestCase
 from kitsune.wiki.models import Document
@@ -611,3 +614,32 @@ def test_frameless_link(self):
         img = img_a("img")
         assert "frameless" in img.attr("class")
         self.assertEqual("/en-US/kb/installing-firefox", img_a.attr("href"))
+
+
+class AllowedAttributesTests(TestCase):
+    PAYLOAD = '<div id="x" class="c" data-target="#btn" data-modal="true">content</div>'
+
+    def test_base_strips_dangerous_div_attributes(self):
+        """BASE_ALLOWED_ATTRIBUTES removes id, data-target and data-modal from divs."""
+        result = pq(wiki_to_html(self.PAYLOAD, attributes=BASE_ALLOWED_ATTRIBUTES))("div")
+        self.assertIsNone(result.attr("id"))
+        self.assertIsNone(result.attr("data-target"))
+        self.assertIsNone(result.attr("data-modal"))
+        self.assertEqual("c", result.attr("class"))
+
+    def test_wiki_attributes_preserve_div_id(self):
+        """ALLOWED_ATTRIBUTES (wiki) keeps id on divs."""
+        result = pq(wiki_to_html(self.PAYLOAD, attributes=ALLOWED_ATTRIBUTES))("div")
+        self.assertEqual("x", result.attr("id"))
+
+    def test_base_strips_heading_ids(self):
+        """BASE_ALLOWED_ATTRIBUTES removes id from headings."""
+        result = pq(wiki_to_html('<h2 id="sec">Title</h2>', attributes=BASE_ALLOWED_ATTRIBUTES))(
+            "h2"
+        )
+        self.assertIsNone(result.attr("id"))
+
+    def test_wiki_attributes_preserve_heading_ids(self):
+        """ALLOWED_ATTRIBUTES (wiki) generates anchor ids on headings."""
+        result = pq(wiki_to_html('<h2 id="sec">Title</h2>', attributes=ALLOWED_ATTRIBUTES))("h2")
+        self.assertIsNotNone(result.attr("id"))
diff --git a/kitsune/users/jinja2/users/edit_profile.html b/kitsune/users/jinja2/users/edit_profile.html
@@ -98,7 +98,7 @@ <h2 class="sumo-callout-heading">{{ title }}</h2>
   {% endif %}
 
   {% if request.user == profile.user %}
-    <div class="mzp-u-modal-content kbox" title="{{ _('Are you sure') }}" id="delete-profile">
+    <div class="mzp-u-modal-content kbox" title="{{ _('Are you sure') }}" data-modal-id="delete-profile">
       <form method="post" action="{{ url('users.close_account') }}">
         {% csrf_token %}
         <p>
diff --git a/kitsune/wiki/jinja2/wiki/includes/flag_form.html b/kitsune/wiki/jinja2/wiki/includes/flag_form.html
@@ -1,6 +1,6 @@
 {% macro flag_form(post_target, identifier, content_moderation=False) %}
   <a href="javascript:;" data-sumo-modal="report-abuse-{{ identifier }}">{{ _('Report Abuse') }}</a>
-  <section id="report-abuse-{{ identifier }}" class="mzp-u-modal-content">
+  <section data-modal-id="report-abuse-{{ identifier }}" class="mzp-u-modal-content">
     <h2 class="sumo-page-subheading">{{ _('Report this') }}</h2>
     <form action="{{ post_target }}" method="post">
       {% csrf_token %}
diff --git a/kitsune/wiki/jinja2/wiki/includes/ready_for_l10n_modal.html b/kitsune/wiki/jinja2/wiki/includes/ready_for_l10n_modal.html
@@ -1,5 +1,5 @@
 <div>{# Dummy wrapper so modal doesn't get shown by details/summary code #}
-  <div class="mzp-u-modal-content" id="ready-for-l10n-modal" title="{{ _('Mark as Ready for Localization') }}">
+  <div class="mzp-u-modal-content" data-modal-id="ready-for-l10n-modal" title="{{ _('Mark as Ready for Localization') }}">
     {% csrf_token %}
     <div>
       <p>
