From 4546653b5a0f347acbed3068f87cc1096279990d Mon Sep 17 00:00:00 2001
From: catborise <catborise@gmail.com>
Date: Fri, 2 Oct 2026 18:29:11 +0300
Subject: [PATCH] fix: serve JSON endpoints as application/json and build DOM
 safely

Several views returned json.dumps() output through HttpResponse, i.e. as
text/html, and guess_mac_address reflects the URL's vname into it
(reflected XSS). Templates filled the log table and volume dropdowns by
concatenating server data into HTML strings, and log messages contain
user-entered snapshot/disk/clone names (stored XSS that runs when a
superuser opens the Logs tab).

- Use JsonResponse in guess_mac_address, random_mac_address,
  guess_clone_name, sshkeys, storages.get_volumes,
  datasource.os_metadata_json and logs.vm_logs; sshkeys?plain=true is
  served as text/plain.
- stats_tab.html builds log rows with .text(); instance.html and
  create_instance_w2.html build <option> elements with .val().text().
---
 datasource/views.py                          |   5 +-
 instances/templates/create_instance_w2.html  |   6 +-
 instances/templates/instance.html            |   7 +-
 instances/templates/instances/stats_tab.html |  17 +--
 instances/test_xss.py                        | 111 +++++++++++++++++++
 instances/views.py                           |  13 +--
 logs/views.py                                |   5 +-
 storages/views.py                            |   5 +-
 8 files changed, 140 insertions(+), 29 deletions(-)
 create mode 100644 instances/test_xss.py

diff --git a/datasource/views.py b/datasource/views.py
index bbdd27a6..e888fb54 100644
--- a/datasource/views.py
+++ b/datasource/views.py
@@ -1,9 +1,8 @@
-import json
 import socket
 
 from accounts.models import UserInstance, UserSSHKey
 from computes.models import Compute
-from django.http import Http404, HttpResponse
+from django.http import Http404, HttpResponse, JsonResponse
 from django.shortcuts import get_object_or_404, render
 from libvirt import libvirtError
 from vrtManager.instance import wvmInstance
@@ -32,7 +31,7 @@ def os_metadata_json(request, version):
         ip = get_client_ip(request)
         hostname = get_hostname_by_ip(ip)
         response = {"uuid": OS_UUID, "hostname": hostname}
-        return HttpResponse(json.dumps(response))
+        return JsonResponse(response)
     else:
         err = "Invalid version: %(version)s" % {"version": version}
         raise Http404(err)
diff --git a/instances/templates/create_instance_w2.html b/instances/templates/create_instance_w2.html
index 624e0b06..c9593c8e 100644
--- a/instances/templates/create_instance_w2.html
+++ b/instances/templates/create_instance_w2.html
@@ -967,7 +967,7 @@
         $.getJSON(get_vol_url, function (data) {
             $("#image-control").find('option').remove();
             $.each(data['vols'], function(i, item) {
-                $("#image-control").append('<option value=' + item +'>' + item + '</option>');
+                $("#image-control").append($("<option>").val(item).text(item));
             });
             $('#image-control').multiselect('rebuild');
         });
@@ -978,7 +978,7 @@
         $.getJSON(get_vol_url, function (data) {
             $("#template").find('option').remove();
             $.each(data['vols'], function(i, item) {
-                $("#template").append('<option value=' + item +'>' + item + '</option>');
+                $("#template").append($("<option>").val(item).text(item));
             });
         });
         $("#template").removeAttr("disabled");
@@ -993,7 +993,7 @@
         $.getJSON(get_diskBus_url, function (data) {
             $("#bus" + dev_idx).find('option').remove();
             $.each(data['bus'], function(i, item) {
-                $("#bus" + dev_idx).append('<option value=' + item +'>' + item + '</option>');
+                $("#bus" + dev_idx).append($("<option>").val(item).text(item));
             });
         });
     }
diff --git a/instances/templates/instance.html b/instances/templates/instance.html
index 7d1ae8e5..bbaa2e36 100644
--- a/instances/templates/instance.html
+++ b/instances/templates/instance.html
@@ -150,13 +150,16 @@
             $("#vols").removeAttr("disabled");
 
             $.each(data['vols'], function(i, item) {
-                $("#vols").append('<option value=' + item +'>' + item + '</option>');
+                $("#vols").append($("<option>").val(item).text(item));
             })
         });
 
         var sto_drop = document.getElementById('select_storage');
         sto_drop.value = pool;
-        sto_drop.innerHTML = pool + "<span class=\"caret\"></span>";
+        sto_drop.textContent = pool;
+        var caret = document.createElement("span");
+        caret.className = "caret";
+        sto_drop.appendChild(caret);
 
         var sto_input = document.getElementById('selected_storage');
         sto_input.setAttribute('value', pool);
diff --git a/instances/templates/instances/stats_tab.html b/instances/templates/instances/stats_tab.html
index 768fc8be..965163ea 100644
--- a/instances/templates/instances/stats_tab.html
+++ b/instances/templates/instances/stats_tab.html
@@ -105,15 +105,16 @@
         // TODO
         logurl = "{% url 'vm_logs' 1 %}".replace(1, vname);
         $.getJSON(logurl, function(data) {
-            var logs = "";
-            $.each(data, function(id) {
-                row = data[id];
-                // console.log(row);
-                logs += '<tr><td style="width:150px">'+row['date']+'</td>';
-                logs += '<td>'+row['user']+'</td>';
-                logs += '<td>'+row['message']+'</td></tr>';
+            // Log messages contain user-entered names (snapshots, disks, clones);
+            // insert them as text, never as HTML.
+            var tbody = $("#logs_table > tbody").empty();
+            $.each(data, function(id, row) {
+                $("<tr>")
+                    .append($("<td>").css("width", "150px").text(row['date']))
+                    .append($("<td>").text(row['user']))
+                    .append($("<td>").text(row['message']))
+                    .appendTo(tbody);
             });
-            $("#logs_table > tbody").html(logs);
         });
     }
 </script>
diff --git a/instances/test_xss.py b/instances/test_xss.py
new file mode 100644
index 00000000..3a8537c8
--- /dev/null
+++ b/instances/test_xss.py
@@ -0,0 +1,111 @@
+"""
+Regression tests for ROADMAP Wave 0 item 3 (S-06, S-07): JSON endpoints must
+not be served as text/html (reflected XSS), and templates must not build HTML
+by concatenating server data that contains user-controlled names (stored XSS).
+"""
+from pathlib import Path
+from unittest.mock import patch
+
+from django.contrib.auth import get_user_model
+from django.test import TestCase
+from django.urls import reverse
+
+from accounts.models import UserInstance, UserSSHKey
+from computes.models import Compute
+from instances.models import Instance
+
+ROOT = Path(__file__).resolve().parent.parent
+PAYLOAD = "<script>alert(1)</script>"
+
+
+class JsonEndpointContentTypeTestCase(TestCase):
+    def setUp(self):
+        User = get_user_model()
+        self.superuser = User.objects.create_superuser(
+            username="xss_super", email="xss_super@example.com", password="password"
+        )
+        self.compute = Compute.objects.create(
+            name="xss-compute", hostname="127.0.0.1", login="root", password="", type=1
+        )
+        self.instance = Instance.objects.create(
+            compute=self.compute,
+            name="xss-vm",
+            uuid="99999999-8888-7777-6666-555555555555",
+        )
+        UserInstance.objects.create(instance=self.instance, user=self.superuser)
+        UserSSHKey.objects.create(
+            user=self.superuser,
+            keyname="k",
+            keypublic=f"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAI {PAYLOAD}",
+        )
+        self.client.force_login(self.superuser)
+
+    def assertJson(self, response):
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response["Content-Type"], "application/json")
+
+    def test_guess_mac_address_reflects_vname_as_json(self):
+        res = self.client.get(
+            reverse("instances:guess_mac_address", args=["<img src=x onerror=alert(1)>"])
+        )
+        self.assertJson(res)
+
+    def test_random_mac_address_is_json(self):
+        self.assertJson(self.client.get(reverse("instances:random_mac_address")))
+
+    def test_guess_clone_name_is_json(self):
+        self.assertJson(self.client.get(reverse("instances:guess_clone_name")))
+
+    def test_sshkeys_is_json(self):
+        self.assertJson(
+            self.client.get(reverse("instances:sshkeys", args=[self.instance.id]))
+        )
+
+    def test_sshkeys_plain_is_text_plain(self):
+        res = self.client.get(
+            reverse("instances:sshkeys", args=[self.instance.id]) + "?plain=true"
+        )
+        self.assertEqual(res.status_code, 200)
+        self.assertTrue(res["Content-Type"].startswith("text/plain"))
+
+    def test_storage_volumes_is_json(self):
+        with patch("storages.views.wvmStorage") as mock_storage:
+            mock_storage.return_value.get_volumes.return_value = [PAYLOAD]
+            res = self.client.get(
+                reverse("volumes", args=[self.compute.id, "default"])
+            )
+        self.assertJson(res)
+
+    def test_datasource_metadata_is_json(self):
+        self.assertJson(
+            self.client.get(reverse("ds_openstack_metadata", args=["latest"]))
+        )
+
+    def test_vm_logs_is_json(self):
+        self.assertJson(self.client.get(reverse("vm_logs", args=[self.instance.name])))
+
+
+class TemplateHtmlConcatenationTestCase(TestCase):
+    """
+    The log table and the volume dropdown are filled by JavaScript from JSON
+    that contains user-entered names. They must use DOM APIs (.text()/.val())
+    instead of concatenating the values into an HTML string.
+    """
+
+    def _read(self, relative):
+        return (ROOT / relative).read_text()
+
+    def test_logs_table_does_not_concatenate_log_fields_into_html(self):
+        src = self._read("instances/templates/instances/stats_tab.html")
+        for field in ("date", "user", "message"):
+            self.assertNotIn(f"+row['{field}']+", src)
+        self.assertNotIn('$("#logs_table > tbody").html(', src)
+
+    def test_volume_dropdown_does_not_concatenate_volume_names_into_html(self):
+        src = self._read("instances/templates/instance.html")
+        self.assertNotIn("'<option value=' + item", src)
+        self.assertNotIn('pool + "<span', src)
+
+    def test_create_wizard_volume_lists_do_not_concatenate_volume_names_into_html(self):
+        src = self._read("instances/templates/create_instance_w2.html")
+        self.assertNotIn("'<option value=' + item", src)
diff --git a/instances/views.py b/instances/views.py
index 1c45f9d3..c66c7b00 100755
--- a/instances/views.py
+++ b/instances/views.py
@@ -261,13 +261,13 @@ def guess_mac_address(request, vname):
     if not mac:
         mac = utils.get_random_mac_address()
     data["mac"] = mac
-    return HttpResponse(json.dumps(data))
+    return JsonResponse(data)
 
 
 def random_mac_address(request):
     data = dict()
     data["mac"] = utils.get_random_mac_address()
-    return HttpResponse(json.dumps(data))
+    return JsonResponse(data)
 
 
 def guess_clone_name(request):
@@ -284,8 +284,8 @@ def guess_clone_name(request):
                     fqdn = line.split(" ")[1]
                     hostname = fqdn.split(".")[0]
                     if hostname.startswith(prefix) and hostname not in instance_names:
-                        return HttpResponse(json.dumps({"name": hostname}))
-    return HttpResponse(json.dumps({}))
+                        return JsonResponse({"name": hostname})
+    return JsonResponse({})
 
 
 def check_instance(request, vname):
@@ -313,9 +313,8 @@ def sshkeys(request, pk):
     if request.GET.get("plain", ""):
         response = "\n".join(instance_keys)
         response += "\n"
-    else:
-        response = json.dumps(instance_keys)
-    return HttpResponse(response)
+        return HttpResponse(response, content_type="text/plain; charset=utf-8")
+    return JsonResponse(instance_keys, safe=False)
 
 
 def get_safe_redirect(request, default=None):
diff --git a/logs/views.py b/logs/views.py
index dbb750c6..2671cdfe 100644
--- a/logs/views.py
+++ b/logs/views.py
@@ -1,7 +1,6 @@
-import json
 import logging
 
-from django.http import HttpResponse
+from django.http import JsonResponse
 
 from admin.decorators import superuser_only
 from instances.models import Instance
@@ -42,4 +41,4 @@ def vm_logs(request, vname):
         log["date"] = l.date.strftime("%x %X")
         logs.append(log)
 
-    return HttpResponse(json.dumps(logs))
+    return JsonResponse(logs, safe=False)
diff --git a/storages/views.py b/storages/views.py
index 8f18ebc9..7d031ab9 100644
--- a/storages/views.py
+++ b/storages/views.py
@@ -1,4 +1,3 @@
-import json
 import os
 import re
 
@@ -6,7 +5,7 @@ from admin.decorators import superuser_only
 from appsettings.settings import app_settings
 from computes.models import Compute
 from django.contrib import messages
-from django.http import HttpResponse, HttpResponseRedirect, JsonResponse
+from django.http import HttpResponseRedirect, JsonResponse
 from django.shortcuts import get_object_or_404, redirect, render
 from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
@@ -337,4 +336,4 @@ def get_volumes(request, compute_id, pool):
         data["vols"] = sorted(conn.get_volumes())
     except libvirtError:
         pass
-    return HttpResponse(json.dumps(data))
+    return JsonResponse(data)