Skip to content

Potential fix for code scanning alert no. 2: Incomplete multi-character sanitization #339

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.

Sign up for GitHub

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 1 commit into
base: main
Choose a base branch
from
Draft

Potential fix for code scanning alert no. 2: Incomplete multi-character sanitization #339

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/runtime/server/og-image/templates/html.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,11 @@
})

// need to remove ALL script tags from the html
html = html.replaceAll(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, '')
let previousHtml;
do {
previousHtml = html;
html = html.replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, '');

Check failure

Code scanning / CodeQL

Bad HTML filtering regexp High

This regular expression does not match script end tags like </script >.

Copilot Autofix

AI 7 months ago

The best way to fix the problem is to use a well-tested HTML sanitization library instead of relying on a custom regular expression. This approach ensures that all edge cases and variations in HTML tags are properly handled, reducing the risk of security vulnerabilities.

To implement this fix, we will:

  1. Install the sanitize-html library.
  2. Replace the custom regular expression with a call to the sanitize-html library to remove all <script> tags from the HTML.
Suggested changeset 2
src/runtime/server/og-image/templates/html.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/runtime/server/og-image/templates/html.ts b/src/runtime/server/og-image/templates/html.ts
--- a/src/runtime/server/og-image/templates/html.ts
+++ b/src/runtime/server/og-image/templates/html.ts
@@ -8,2 +8,3 @@
 import { applyEmojis } from '../satori/transforms/emojis'
+import sanitizeHtml from 'sanitize-html'
 
@@ -116,7 +117,5 @@
   // need to remove ALL script tags from the html
-  let previousHtml;
-  do {
-    previousHtml = html;
-    html = html.replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, '');
-  } while (html !== previousHtml);
+  html = sanitizeHtml(html, {
+    allowedTags: sanitizeHtml.defaults.allowedTags.filter(tag => tag !== 'script'),
+  });
 
EOF
@@ -8,2 +8,3 @@
import { applyEmojis } from '../satori/transforms/emojis'
import sanitizeHtml from 'sanitize-html'

@@ -116,7 +117,5 @@
// need to remove ALL script tags from the html
let previousHtml;
do {
previousHtml = html;
html = html.replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, '');
} while (html !== previousHtml);
html = sanitizeHtml(html, {
allowedTags: sanitizeHtml.defaults.allowedTags.filter(tag => tag !== 'script'),
});

package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/package.json b/package.json
--- a/package.json
+++ b/package.json
@@ -99,3 +99,4 @@
     "unwasm": "^0.3.9",
-    "yoga-wasm-web": "^0.3.3"
+    "yoga-wasm-web": "^0.3.3",
+    "sanitize-html": "^2.14.0"
   },
EOF
@@ -99,3 +99,4 @@
"unwasm": "^0.3.9",
"yoga-wasm-web": "^0.3.3"
"yoga-wasm-web": "^0.3.3",
"sanitize-html": "^2.14.0"
},
This fix introduces these dependencies
Package Version Security advisories
sanitize-html (npm) 2.14.0 None
Copilot is powered by AI and may make mistakes. Always verify output.
} while (html !== previousHtml);

const headChunk = await renderSSRHead(head)
return `<!DOCTYPE html>
Expand Down