Skip to content

[feature] 7 #130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 78 commits into
base: master
Choose a base branch
from
Draft

[feature] 7 #130

wants to merge 78 commits into from

Conversation

himn1
Copy link
Collaborator

@himn1 himn1 commented Apr 11, 2025

No description provided.


const renderRoom = (room) => {
const tbody = document.querySelector("#rooms_list tbody");
tbody.insertAdjacentHTML("beforeend", roomTmpl(room));

Check warning

Code scanning / CodeQL

DOM text reinterpreted as HTML Medium test

DOM text
is reinterpreted as HTML without escaping meta-characters.

Copilot Autofix

AI 16 days ago

To fix the issue, we need to ensure that any data used to generate HTML is properly sanitized or escaped to prevent XSS vulnerabilities. The best approach is to escape the dynamic content in the roomTmpl function before it is inserted into the DOM. This can be achieved by creating a utility function to escape HTML special characters (<, >, &, ", ') and applying it to all dynamic values in the template.

Changes to be made:

  1. Add a utility function to escape HTML special characters.
  2. Update the roomTmpl function to use the escape function for all dynamic content.

Suggested changeset 1
test/dummy/frontend/js/views/user/rooms/List.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/test/dummy/frontend/js/views/user/rooms/List.js b/test/dummy/frontend/js/views/user/rooms/List.js
--- a/test/dummy/frontend/js/views/user/rooms/List.js
+++ b/test/dummy/frontend/js/views/user/rooms/List.js
@@ -7,7 +7,16 @@
 
+const escapeHTML = (str) => {
+  return String(str)
+    .replace(/&/g, "&amp;")
+    .replace(/</g, "&lt;")
+    .replace(/>/g, "&gt;")
+    .replace(/"/g, "&quot;")
+    .replace(/'/g, "&#39;");
+};
+
 const roomTmpl = ({ id, name, members_count, joined }) => {
   return `
-  <tr id="room_${id}">
-    <td>${name}</td>
-    <td class="members">${members_count}</td>
+  <tr id="room_${escapeHTML(id)}">
+    <td>${escapeHTML(name)}</td>
+    <td class="members">${escapeHTML(members_count)}</td>
     <td>
@@ -16,3 +25,3 @@
         data-method="patch"
-        href="${joined ? `/user/rooms/${id}/leave` : `/user/rooms/${id}/join`}"
+        href="${joined ? `/user/rooms/${escapeHTML(id)}/leave` : `/user/rooms/${escapeHTML(id)}/join`}"
       >
@@ -25,3 +34,3 @@
         data-confirm="R U sure?"
-        href="/user/rooms/${id}"
+        href="/user/rooms/${escapeHTML(id)}"
       >
EOF
@@ -7,7 +7,16 @@

const escapeHTML = (str) => {
return String(str)
.replace(/&/g, "&amp;")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;")
.replace(/"/g, "&quot;")
.replace(/'/g, "&#39;");
};

const roomTmpl = ({ id, name, members_count, joined }) => {
return `
<tr id="room_${id}">
<td>${name}</td>
<td class="members">${members_count}</td>
<tr id="room_${escapeHTML(id)}">
<td>${escapeHTML(name)}</td>
<td class="members">${escapeHTML(members_count)}</td>
<td>
@@ -16,3 +25,3 @@
data-method="patch"
href="${joined ? `/user/rooms/${id}/leave` : `/user/rooms/${id}/join`}"
href="${joined ? `/user/rooms/${escapeHTML(id)}/leave` : `/user/rooms/${escapeHTML(id)}/join`}"
>
@@ -25,3 +34,3 @@
data-confirm="R U sure?"
href="/user/rooms/${id}"
href="/user/rooms/${escapeHTML(id)}"
>
Copilot is powered by AI and may make mistakes. Always verify output.
const reRenderRoom = (roomId) => {
const room = rooms.find(r => r.id === roomId);
const node = document.getElementById(`room_${roomId}`);
node.innerHTML = roomTmpl(room);

Check warning

Code scanning / CodeQL

DOM text reinterpreted as HTML Medium test

DOM text
is reinterpreted as HTML without escaping meta-characters.

Copilot Autofix

AI 16 days ago

To fix the issue, we need to ensure that any data inserted into the DOM is properly sanitized or escaped to prevent XSS attacks. Instead of using innerHTML to insert the HTML string generated by roomTmpl, we can use DOM manipulation methods to create and append elements safely. This approach avoids interpreting the data as HTML and ensures that any special characters are treated as plain text.

The changes involve:

  1. Refactoring the roomTmpl function to return a DOM element instead of an HTML string.
  2. Updating the renderRoom and reRenderRoom functions to use the new roomTmpl implementation.

Suggested changeset 1
test/dummy/frontend/js/views/user/rooms/List.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/test/dummy/frontend/js/views/user/rooms/List.js b/test/dummy/frontend/js/views/user/rooms/List.js
--- a/test/dummy/frontend/js/views/user/rooms/List.js
+++ b/test/dummy/frontend/js/views/user/rooms/List.js
@@ -8,26 +8,35 @@
 const roomTmpl = ({ id, name, members_count, joined }) => {
-  return `
-  <tr id="room_${id}">
-    <td>${name}</td>
-    <td class="members">${members_count}</td>
-    <td>
-      <a
-        rel="nofollow"
-        data-method="patch"
-        href="${joined ? `/user/rooms/${id}/leave` : `/user/rooms/${id}/join`}"
-      >
-        ${joined ? 'Leave' : 'Join'}
-      </a>
-      |
-      <a
-        rel="nofollow"
-        data-method="delete"
-        data-confirm="R U sure?"
-        href="/user/rooms/${id}"
-      >
-        Destroy
-      </a>
-    </td>
-  </tr>
-  `;
+  const tr = document.createElement("tr");
+  tr.id = `room_${id}`;
+
+  const nameTd = document.createElement("td");
+  nameTd.textContent = name;
+  tr.appendChild(nameTd);
+
+  const membersTd = document.createElement("td");
+  membersTd.className = "members";
+  membersTd.textContent = members_count;
+  tr.appendChild(membersTd);
+
+  const actionsTd = document.createElement("td");
+
+  const joinLeaveLink = document.createElement("a");
+  joinLeaveLink.rel = "nofollow";
+  joinLeaveLink.setAttribute("data-method", "patch");
+  joinLeaveLink.href = joined ? `/user/rooms/${id}/leave` : `/user/rooms/${id}/join`;
+  joinLeaveLink.textContent = joined ? "Leave" : "Join";
+  actionsTd.appendChild(joinLeaveLink);
+
+  actionsTd.appendChild(document.createTextNode(" | "));
+
+  const destroyLink = document.createElement("a");
+  destroyLink.rel = "nofollow";
+  destroyLink.setAttribute("data-method", "delete");
+  destroyLink.setAttribute("data-confirm", "R U sure?");
+  destroyLink.href = `/user/rooms/${id}`;
+  destroyLink.textContent = "Destroy";
+  actionsTd.appendChild(destroyLink);
+
+  tr.appendChild(actionsTd);
+  return tr;
 };
@@ -40,3 +49,3 @@
   const tbody = document.querySelector("#rooms_list tbody");
-  tbody.insertAdjacentHTML("beforeend", roomTmpl(room));
+  tbody.appendChild(roomTmpl(room));
 }
@@ -45,4 +54,5 @@
   const room = rooms.find(r => r.id === roomId);
-  const node = document.getElementById(`room_${roomId}`);
-  node.innerHTML = roomTmpl(room);
+  const oldNode = document.getElementById(`room_${roomId}`);
+  const newNode = roomTmpl(room);
+  oldNode.replaceWith(newNode);
 };
EOF
Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants