Skip to content

Commit 88b3a53

Browse files
[APS-19078] fix CI: replace regex HTML strip with sanitize-html (CodeQL)
CodeQL flagged the `<script|iframe|object|embed>` regex in sanitizeCss as "incomplete multi-character sanitization" (high severity x2 — src + dist). A single-pass replace is bypassable by nested patterns like `<scr<script>ipt>`, which collapse to `<script>` after one substitution. Fix: strip ALL HTML tags from the rich-CSS payload via sanitize-html with allowedTags: [] (the library iterates internally and is not bypassable). The CSS-function regexes for `expression(...)` and `url(javascript:...)` remain — they target CSS syntax, not HTML, and CodeQL did not flag them. Verified: - 18/18 unit tests pass (no regression) - Sanity script confirms `<scr<script>ipt>` no longer leaves a `<script` substring in the output - Flagged regex literal absent from rebuilt dist/index.js - Lint clean Resolves: CodeQL failure on PR #85
1 parent 67f66ea commit 88b3a53

2 files changed

Lines changed: 16 additions & 8 deletions

File tree

browserstack-report-action/dist/index.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51127,13 +51127,17 @@ const HTML_SANITIZE_OPTIONS = {
5112751127
};
5112851128

5112951129
// CSS sanitizer: strip JS-execution hooks (expression(), url(javascript:...))
51130-
// from the rich-CSS payload before embedding into <style>.
51130+
// from the rich-CSS payload before embedding into <style>. HTML tags are
51131+
// stripped via sanitize-html (allowedTags: []) rather than a regex — a
51132+
// regex replace is single-pass and bypassable by nested patterns like
51133+
// `<scr<script>ipt>`, which CodeQL flags as "incomplete multi-character
51134+
// sanitization". sanitize-html iterates internally and is safe.
5113151135
function sanitizeCss(css) {
5113251136
if (!css || typeof css !== 'string') return '';
51133-
return css
51137+
const stripped = sanitizeHtml(css, { allowedTags: [], allowedAttributes: {} });
51138+
return stripped
5113451139
.replace(/expression\s*\([^)]*\)/gi, '')
51135-
.replace(/url\s*\(\s*['"]?\s*javascript:[^)]*\)/gi, '')
51136-
.replace(/<\/?(script|iframe|object|embed)[^>]*>/gi, '');
51140+
.replace(/url\s*\(\s*['"]?\s*javascript:[^)]*\)/gi, '');
5113751141
}
5113851142

5113951143
class ReportProcessor {

browserstack-report-action/src/services/ReportProcessor.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,17 @@ const HTML_SANITIZE_OPTIONS = {
2929
};
3030

3131
// CSS sanitizer: strip JS-execution hooks (expression(), url(javascript:...))
32-
// from the rich-CSS payload before embedding into <style>.
32+
// from the rich-CSS payload before embedding into <style>. HTML tags are
33+
// stripped via sanitize-html (allowedTags: []) rather than a regex — a
34+
// regex replace is single-pass and bypassable by nested patterns like
35+
// `<scr<script>ipt>`, which CodeQL flags as "incomplete multi-character
36+
// sanitization". sanitize-html iterates internally and is safe.
3337
function sanitizeCss(css) {
3438
if (!css || typeof css !== 'string') return '';
35-
return css
39+
const stripped = sanitizeHtml(css, { allowedTags: [], allowedAttributes: {} });
40+
return stripped
3641
.replace(/expression\s*\([^)]*\)/gi, '')
37-
.replace(/url\s*\(\s*['"]?\s*javascript:[^)]*\)/gi, '')
38-
.replace(/<\/?(script|iframe|object|embed)[^>]*>/gi, '');
42+
.replace(/url\s*\(\s*['"]?\s*javascript:[^)]*\)/gi, '');
3943
}
4044

4145
class ReportProcessor {

0 commit comments

Comments
 (0)