[LTM] Fix quoting of query parameters in list view (#5647)

* Add proper quoting of object-list filters

* Add unit test

* Change fragment

* Fix removal of filter params

* Use location.assign rather than location.replace, close modal on form submit

changes/5647.security

@@ -0,0 +1 @@
+Fixed a reflected-XSS vulnerability ([GHSA-jxgr-gcj5-cqqg](https://github.com/nautobot/nautobot/security/advisories/GHSA-jxgr-gcj5-cqqg)) in object-list view rendering of user-provided query parameters.

nautobot/core/templates/generic/object_list.html

@@ -68,7 +68,7 @@ <h1>{% block title %}{{ title }}{% endblock %}</h1>
                     class="remove-filter-param"
                     title="Remove all items"
                     data-field-type="parent"
-                    data-field-value={{ field.name }}
+                    data-field-value="{{ field.name }}"
             >×</span>
             <ul class="filter-selection-rendered">
                 {% for value in field.values %}
@@ -79,8 +79,8 @@ <h1>{% block title %}{{ title }}{% endblock %}</h1>
                     <span
                             class="filter-selection-choice-remove remove-filter-param"
                             data-field-type="child"
-                            data-field-parent={{ field.name }}
-                            data-field-value={{ value.name }}
+                            data-field-parent="{{ field.name }}"
+                            data-field-value="{{ value.name }}"
                     >×</span>{{ value.display }}
                 </li>
                 {% endfor %}
@@ -185,18 +185,19 @@ <h1>{% block title %}{{ title }}{% endblock %}</h1>
 
     // Remove applied filters
     $(".remove-filter-param").on("click", function(){
-        let query_params = location.search;
+        let query_params = new URLSearchParams(location.search);
         let type = $(this).attr("data-field-type");
         let field_value = $(this).attr("data-field-value");
-        let query_string = location.search.substr(1).split("&");
 
         if (type === "parent") {
-            query_string = query_string.filter(item => item.search(field_value) < 0);
+            // Remove all instances of this query param
+            query_params.delete(field_value);
         } else {
+            // Remove this specific instance of this query param
             let parent = $(this).attr("data-field-parent");
-            query_string = query_string.filter(item => item.search(parent + "=" + field_value) < 0)
+            query_params.delete(parent, field_value);
         }
-        location.replace("?" + query_string.join("&"))
+        location.assign("?" + query_params);
     })
 
     // On submit of filter form
@@ -210,10 +211,18 @@ <h1>{% block title %}{{ title }}{% endblock %}</h1>
         q_field_phantom.val(q_field.val())
         dynamic_form.append(q_field_phantom);
 
-        let search_query = $("#dynamic-filter-form, #default-filter form").serialize()
-        // Remove duplicates generated by filter merging on both dynamic and default filter forms.
-        search_query = "&" + search_query.replace(/([^&]+=[^&]+&)(?=.*\1)/g, "")
-        location.replace("?" + search_query)
+        // Get the serialized data from the forms and:
+        // 1) filter out query_params which values are empty e.g. ?sam=&dan=2 becomes ?dan=2
+        // 2) combine the two forms into a single set of data without duplicate entries
+        let search_query = new URLSearchParams()
+        let dynamic_query = new URLSearchParams(new FormData(document.getElementById("dynamic-filter-form")));
+        dynamic_query.forEach((value, key) => { if (value != "") { search_query.append(key, value); }});
+        let default_query = new URLSearchParams(new FormData(document.getElementById("default-filter").firstElementChild));
+        default_query.forEach((value, key) => {
+            if (value != "" && !search_query.has(key, value)) { search_query.append(key, value); }
+        });
+        $("#FilterForm_modal").modal("hide");
+        location.assign("?" + search_query);
     })
 
     // On submit of filter search form

nautobot/core/tests/test_views.py

@@ -163,6 +163,25 @@ def test_support_for_both_default_and_dynamic_filter_form_in_ui(self):
             response.content.decode(response.charset),
         )
 
+    def test_filtering_crafted_query_params(self):
+        """Test for reflected-XSS vulnerability GHSA-jxgr-gcj5-cqqg."""
+        self.add_permissions("dcim.view_location")
+        query_param = "?location_type=1 onmouseover=alert('hi') foo=bar"
+        url = reverse("dcim:location_list") + query_param
+        response = self.client.get(url)
+        self.assertHttpStatus(response, 200)
+        response_content = response.content.decode(response.charset)
+        # The important thing here is that the data-field-parent and data-field-value are correctly quoted
+        self.assertInHTML(
+            """
+<span class="filter-selection-choice-remove remove-filter-param"
+      data-field-type="child"
+      data-field-parent="location_type"
+      data-field-value="1 onmouseover=alert(&#x27;hi&#x27;) foo=bar"
+>×</span>""",  # noqa: RUF001 - ambiguous-unicode-character-string
+            response_content,
+        )
+
 
 class ForceScriptNameTestcase(TestCase):
     """Basic test to assert that `settings.FORCE_SCRIPT_NAME` works as intended."""