Skip to content

Conversation

@Praj-na
Copy link
Contributor

@Praj-na Praj-na commented Jul 15, 2025

Potential fix for https://github.com/UBC-CIC/Legal-Aid-Tool/security/code-scanning/8

To fix the issue, the sanitization of the parsed Markdown content should be done using a robust and well-tested library rather than relying on a custom regular expression. The DOMPurify library is already imported in the file and is specifically designed to sanitize HTML content effectively. By using DOMPurify.sanitize() on the parsed HTML from marked.parse(audioText), we can ensure that all potentially harmful tags and attributes are removed. This eliminates the risk of injection vulnerabilities.


Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…er sanitization

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

const content = marked.parse(audioText).replace(/<[^>]+>/g, "");
const sanitizedHtml = DOMPurify.sanitize(marked.parse(audioText));
const content = sanitizedHtml.replace(/<[^>]+>/g, "");

Check failure

Code scanning / CodeQL

Incomplete multi-character sanitization High

This string may still contain
<script
, which may cause an HTML element injection vulnerability.

Copilot Autofix

AI 6 months ago

To fix this issue:

  1. Replace the regular expression /<[^>]+>/g with a more complete sanitization mechanism to ensure all unsafe HTML content is removed. This can be achieved using DOMPurify's sanitize with a configuration that excludes all HTML tags.
  2. Use DOMPurify to fully sanitize and strip the content, ensuring no residual HTML or tags are present. This avoids potential vulnerabilities caused by relying on a regular expression.

Changes will be made to line 352 to use DOMPurify.sanitize with the ALLOWED_TAGS option set to an empty array, effectively removing all HTML tags.


Suggested changeset 1
frontend/src/pages/CasePage/Transcriptions.jsx

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/frontend/src/pages/CasePage/Transcriptions.jsx b/frontend/src/pages/CasePage/Transcriptions.jsx
--- a/frontend/src/pages/CasePage/Transcriptions.jsx
+++ b/frontend/src/pages/CasePage/Transcriptions.jsx
@@ -348,8 +348,8 @@
   doc.text("Transcription:", margin, y);
   y += 10;
 
-  const sanitizedHtml = DOMPurify.sanitize(marked.parse(audioText));
-  const content = sanitizedHtml.replace(/<[^>]+>/g, "");
+  const sanitizedHtml = DOMPurify.sanitize(marked.parse(audioText), { ALLOWED_TAGS: [], ALLOWED_ATTR: [] });
+  const content = sanitizedHtml;
   const lines = doc.splitTextToSize(content, 180);
 
   for (let i = 0; i < lines.length; i++) {
EOF
@@ -348,8 +348,8 @@
doc.text("Transcription:", margin, y);
y += 10;

const sanitizedHtml = DOMPurify.sanitize(marked.parse(audioText));
const content = sanitizedHtml.replace(/<[^>]+>/g, "");
const sanitizedHtml = DOMPurify.sanitize(marked.parse(audioText), { ALLOWED_TAGS: [], ALLOWED_ATTR: [] });
const content = sanitizedHtml;
const lines = doc.splitTextToSize(content, 180);

for (let i = 0; i < lines.length; i++) {
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