CVE-2024-32979: Insufficient Information (NVD-CWE-noinfo)
value: Nautobot is a Network Source of Truth and Network Automation Platform built as a web application atop the Django Python framework with a PostgreSQL or MySQL database. It was discovered that due to improper handling and escaping of user-provided query parameters, a maliciously crafted Nautobot URL could potentially be used to execute a Reflected Cross-Site Scripting (Reflected XSS) attack against users. All filterable object-list views in Nautobot are vulnerable. This issue has been fixed in Nautobot versions 1.6.20 and 2.2.3. There are no known workarounds for this vulnerability.

CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-05-01T11:15Z
Repository: https://github.com/nautobot/nautobot
Language: Python
Files changed: changes/5646.security, nautobot/core/templates/generic/object_list.html, nautobot/core/tests/test_views.py, nautobot/project-static/js/forms.js
Commit date: 2024-04-30 10:29:41 -0400
Commit message: Fix quoting of query parameters in list view (#5646)

* Add proper quoting of object-list filters

* Add unit test

* Change fragment

* Fix new bug caught by integration tests

* Use location.assign rather than location.replace, close modal on form submit
Diff: diff --git a/changes/5646.security b/changes/5646.security
new file mode 100644
index 0000000000..dbabdaaf52
--- /dev/null
+++ b/changes/5646.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.
diff --git a/nautobot/core/templates/generic/object_list.html b/nautobot/core/templates/generic/object_list.html
index a6ca06fb0e..8c80498dcc 100644
--- a/nautobot/core/templates/generic/object_list.html
+++ b/nautobot/core/templates/generic/object_list.html
@@ -72,23 +72,23 @@
             <b>{{ field.display }}:</b>
             <span
                     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 %}
                 <li
                         class="filter-selection-choice"
                         title="{{ value.name }}"
                 >
                     <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 %}
             </ul>
         </span>
diff --git a/nautobot/core/tests/test_views.py b/nautobot/core/tests/test_views.py
index b99e5966b5..689c0b5a99 100644
--- a/nautobot/core/tests/test_views.py
+++ b/nautobot/core/tests/test_views.py
@@ -240,10 +240,29 @@ class FilterFormsTestCase(TestCase):
         self.assertHttpStatus(response, 200)
         response_content = response.content.decode(response.charset).replace("\n", "")
         self.assertInHTML(locations[0].name, response_content)
         self.assertInHTML(locations[1].name, response_content)
 
+    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."""
 
     @override_settings(
diff --git a/nautobot/project-static/js/forms.js b/nautobot/project-static/js/forms.js
index 20b9cc745b..cfe3e1eb14 100644
--- a/nautobot/project-static/js/forms.js
+++ b/nautobot/project-static/js/forms.js
@@ -630,22 +630,23 @@ function initializeDynamicFilterForm(context){
         lookup_value.attr("name", lookup_type_value);
     })
 
     // Remove applied filters
     this_context.find(".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
     this_context.find("#dynamic-filter-form, #default-filter form").on("submit", function(e){
         e.preventDefault()
@@ -655,16 +656,22 @@ function initializeDynamicFilterForm(context){
         let q_field = $('#id_q')
         let q_field_phantom = $('<input type="hidden" name="q" />')
         q_field_phantom.val(q_field.val())
         dynamic_form.append(q_field_phantom);
 
-        // Get the serialize data from the forms and filter out query_params which values are empty e.g ?sam=&dan=2 becomes dan=2
-        let dynamic_filter_form_query = $("#dynamic-filter-form").serialize().split("&").filter(params => params.split("=")[1]?.length || 0 )
-        let default_filter_form_query = $("#default-filter form").serialize().split("&").filter(params => params.split("=")[1]?.length || 0 )
-        // Union Operation
-        let search_query = [...new Set([...default_filter_form_query, ...dynamic_filter_form_query])].join("&")
-        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
     this_context.find("#search-form").on("submit", function(e){
         // Since the Dynamic Filter Form will already grab my q field, just have it do a majority of the work.

Vulnerable code:                     data-field-value={{ field.name }}
                            data-field-parent={{ field.name }}
                            data-field-value={{ value.name }}
        let query_params = location.search;
        let query_string = location.search.substr(1).split("&");
            query_string = query_string.filter(item => item.search(field_value) < 0);
            query_string = query_string.filter(item => item.search(parent + "=" + field_value) < 0)
        location.replace("?" + query_string.join("&"))
        // Get the serialize data from the forms and filter out query_params which values are empty e.g ?sam=&dan=2 becomes dan=2
        let dynamic_filter_form_query = $("#dynamic-filter-form").serialize().split("&").filter(params => params.split("=")[1]?.length || 0 )
        let default_filter_form_query = $("#default-filter form").serialize().split("&").filter(params => params.split("=")[1]?.length || 0 )
        // Union Operation
        let search_query = [...new Set([...default_filter_form_query, ...dynamic_filter_form_query])].join("&")
        location.replace("?" + search_query)
Fixed code: 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.
                    data-field-value="{{ field.name }}"
                            data-field-parent="{{ field.name }}"
                            data-field-value="{{ value.name }}"
    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,
        )

        let query_params = new URLSearchParams(location.search);
            // Remove all instances of this query param
            query_params.delete(field_value);
            // Remove this specific instance of this query param
            query_params.delete(parent, field_value);
        location.assign("?" + query_params);
        // 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);