fix(review): the SVG drawing check refuses CSS escapes and reads zero-padded character references; the security class reads a file's changed SVG lines joined (GST-911 review pass 1)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-11-02 04:22:06 +00:00
1 parent 8b034914cc
commit d8227c100d
5 files changed
+28 -15

No files matched your search

+6 -3
View File
@@ -81,8 +81,11 @@ const SVG_ELEMENTS = new Set([
]);
/** Markup a drawing may hold besides elements: comments, CDATA, the XML declaration, and a DOCTYPE with no internal subset. */
const SVG_MARKUP = /^<(?:!--|!\[CDATA\[|\?xml\s|!DOCTYPE\s+svg\b[^>[]*>)/i;
/** What a drawing never holds: a named entity beyond the five XML ones, a CSS import, a url() to another document, a CSS expression or binding. */
const SVG_REFUSED = [/&(?!(?:amp|lt|gt|quot|apos);)/, /@import/i, /url\(\s*(?!['"]?\s*#)/i, /expression\s*\(/i, /-moz-binding|\bbehavior\s*:/i];
/**
* What a drawing never holds: a named entity beyond the five XML ones, a backslash (a CSS escape such as @\69mport), a CSS
* import, a url() to another document, a CSS expression or binding.
*/
const SVG_REFUSED = [/&(?!(?:amp|lt|gt|quot|apos);)/, /\\/, /@import/i, /url\(\s*(?!['"]?\s*#)/i, /expression\s*\(/i, /-moz-binding|\bbehavior\s*:/i];
/** An href or src value a drawing may hold: a same-document reference, or a raster image inline. */
const SVG_HREF = /^\s*(?:#|data:image\/(?:png|jpe?g|gif|webp);base64,[A-Za-z0-9+/=\s]*$)/i;
@@ -93,7 +96,7 @@ const SVG_HREF = /^\s*(?:#|data:image\/(?:png|jpe?g|gif|webp);base64,[A-Za-z0-9+
*/
export function svgDrawing(text: string): boolean {
const t = decodeCharRefs(text);
if (SVG_REFUSED.some(r => r.test(t)) || t.split('\n').some(svgActive)) return false;
if (SVG_REFUSED.some(r => r.test(t)) || svgActive(t)) return false;
for (let at = t.indexOf('<'); at !== -1; at = t.indexOf('<', at + 1)) {
if (SVG_MARKUP.test(t.slice(at, at + 512))) continue;
const tag = /^<\/?(?:[A-Za-z_][\w.-]*:)?([A-Za-z_][\w.-]*)/.exec(t.slice(at, at + 256));
+7 -6
View File
@@ -42,14 +42,15 @@ const SECURITY_LINE = [
/\b(?:innerHTML|outerHTML)\s*\+?=/, /\bdangerouslySetInnerHTML\b/, /\bv-html\b/, /\{@html\b/, /\binsertAdjacentHTML\s*\(/, /\bdocument\.write\s*\(/,
];
export const SVG_PATH = /\.svg$/i;
/** An SVG line that can run code, read decoded and again with whitespace removed (java&#9;script: is javascript:). */
/** SVG text that can run code, read decoded and again with whitespace removed (java&#9;script: is javascript:); a file's changed lines are read joined, so a split attribute still counts. */
export const SVG_ACTIVE = [/<(?:[\w.-]+:)?(?:script|foreignObject)\b/i, /(?:^|[\s"'/])on[a-z]+\s*=/i, /(?:java|vb)script:|data:text\/html/i];
/** XML character references decoded (&#106; and &#x6a; are j); an out-of-range one decodes to nothing. */
export const decodeCharRefs = (s: string) => s.replace(/&#(x[0-9a-f]{1,8}|\d{1,10});?/gi, (_, n: string) => {
const c = n[0] === 'x' || n[0] === 'X' ? parseInt(n.slice(1), 16) : Number(n);
/** XML character references decoded, leading zeros included (&#106;, &#x6a; and &#0000000106; are j); an out-of-range one decodes to nothing. */
export const decodeCharRefs = (s: string) => s.replace(/&#(?:[xX]0*([0-9a-fA-F]+)|0*(\d+));?/g, (_, hex: string | undefined, dec: string | undefined) => {
const digits = hex ?? dec!;
const c = digits.length > 8 ? Infinity : hex !== undefined ? parseInt(hex, 16) : Number(dec);
return c <= 0x10ffff ? String.fromCodePoint(c) : '';
});
export const svgActive = (line: string) => { const d = decodeCharRefs(line); const squeezed = d.replace(/[\s\0-\x1f]+/g, ''); return SVG_ACTIVE.some(r => r.test(d) || r.test(squeezed)); };
export const svgActive = (text: string) => { const d = decodeCharRefs(text); const squeezed = d.replace(/[\s\0-\x1f]+/g, ''); return SVG_ACTIVE.some(r => r.test(d) || r.test(squeezed)); };
const DATA_PATH_TOKENS = ['pii', 'gdpr', 'ccpa', 'privacy', 'personal', 'account', 'accounts', 'customer', 'customers', 'cookie', 'cookies', 'persistence', 'storage', 'database'];
const DATA_LINE = [
/\bDELETE\s+FROM\b/, /\b(?:drop\s+(?:table|column|database|schema)|truncate\s+table)\b/i, /\bdelete\s+from\s+[\w."`]+\s*(?:where\b|;|$|["'`)])/i,
@@ -84,11 +85,11 @@ export function matchRisk(i: RiskInput): Map<RiskClass, string> {
if (t.some(x => MONEY_PATH_TOKENS.includes(x))) hit('money', 'money path: ' + p);
if (t.some(x => SECURITY_PATH_TOKENS.includes(x))) hit('security', 'security path: ' + p);
if (t.some(x => DATA_PATH_TOKENS.includes(x))) hit('data', 'data path: ' + p);
if (SVG_PATH.test(p) && [i.added?.[p], i.removed?.[p]].some(l => l && svgActive(l.join('\n')))) hit('security', 'active SVG lines: ' + p);
for (const line of [...(i.added?.[p] ?? []), ...(i.removed?.[p] ?? [])]) {
if (!why.has('money') && MONEY_LINE.some(r => r.test(line))) hit('money', 'money line: ' + p);
if (!why.has('secret') && SECRET_LINE.some(r => r.test(line))) hit('secret', 'secret line: ' + p);
if (!why.has('security') && SECURITY_LINE.some(r => r.test(line))) hit('security', 'security line: ' + p);
if (!why.has('security') && SVG_PATH.test(p) && svgActive(line)) hit('security', 'active SVG line: ' + p);
if (!why.has('data') && DATA_LINE.some(r => r.test(line))) hit('data', 'data line: ' + p);
}
}