From 591a51dcc218830fc29762e604662e62db8b4442 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 18:03:44 -0700 Subject: [PATCH] fix(builder): escape/allowlist all attribute-value sinks incl. numeric/enum props (XSS) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An adversarial pass found 5 Critical XSS sinks where props declared number/enum in TypeScript were interpolated raw into exported HTML attribute values, trusting the type — but nothing enforces it at runtime (AI update_props only validates node_id; deserialized saved state is untyped JSON). Fixed all 5 (NumberCounter data-target, StarRating aria-label, FormContainer method, ContactForm/InputField input type) plus 6 sibling sinks found by an exhaustive audit of every attribute-value interpolation across src/components: a JS-source injection into ContentSlider's inline setInterval script, a prototype-pollution-adjacent allowlist gap in Section's divider-shape lookup, TextareaField rows, Testimonials rating aria-label, HeroSimple textAlign, and MapEmbed zoom. Adds shared sanitizeFormMethod/sanitizeInputType allowlist helpers to utils/escape.ts alongside the existing escapeAttr/safeUrl/cssValue primitives. Every fix is TDD'd: a malicious-value test reproduces the raw injection against the pre-fix code, then passes after the fix. 502 tests green (npx vitest run), tsc + vite build green (npm run build). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../basic/ButtonLink.toHtml.test.ts | 54 +++++++++++++ craft/src/components/basic/ButtonLink.tsx | 4 +- .../components/basic/Divider.toHtml.test.ts | 34 +++++++++ .../components/basic/Footer.toHtml.test.ts | 22 ++++++ craft/src/components/basic/Footer.tsx | 3 +- .../components/basic/Heading.toHtml.test.ts | 18 +++++ craft/src/components/basic/Heading.tsx | 3 +- .../components/basic/HtmlBlock.toHtml.test.ts | 12 +++ .../src/components/basic/Icon.toHtml.test.ts | 46 ++++++++++++ .../src/components/basic/Logo.toHtml.test.ts | 62 +++++++++++++++ .../components/basic/SearchBar.toHtml.test.ts | 19 +++++ .../basic/SocialLinks.toHtml.test.ts | 37 +++++++++ .../components/basic/Spacer.toHtml.test.ts | 26 +++++++ .../basic/StarRating.toHtml.test.ts | 35 +++++++++ craft/src/components/basic/StarRating.tsx | 24 +++++- .../components/basic/TextBlock.toHtml.test.ts | 22 ++++++ craft/src/components/basic/TextBlock.tsx | 3 +- .../forms/ContactForm.toHtml.test.ts | 16 ++++ craft/src/components/forms/ContactForm.tsx | 4 +- .../forms/FormButton.toHtml.test.ts | 25 +++++++ craft/src/components/forms/FormButton.tsx | 3 +- .../forms/FormContainer.toHtml.test.ts | 13 ++++ craft/src/components/forms/FormContainer.tsx | 3 +- .../forms/InputField.toHtml.test.ts | 14 ++++ craft/src/components/forms/InputField.tsx | 4 +- .../forms/SubscribeForm.toHtml.test.ts | 39 ++++++++++ .../forms/TextareaField.toHtml.test.ts | 13 ++++ craft/src/components/forms/TextareaField.tsx | 7 +- .../layout/BackgroundSection.toHtml.test.ts | 58 ++++++++++++++ .../components/layout/Section.toHtml.test.ts | 75 +++++++++++++++++++ craft/src/components/layout/Section.tsx | 14 +++- .../media/ImageBlock.toHtml.test.ts | 34 +++++++++ .../components/media/MapEmbed.toHtml.test.ts | 38 ++++++++++ craft/src/components/media/MapEmbed.tsx | 10 ++- .../media/VideoBlock.toHtml.test.ts | 32 ++++++++ .../sections/ContentSlider.toHtml.test.ts | 20 +++++ .../src/components/sections/ContentSlider.tsx | 17 ++++- .../sections/HeroSimple.toHtml.test.ts | 35 +++++++++ craft/src/components/sections/HeroSimple.tsx | 8 +- .../sections/NumberCounter.toHtml.test.ts | 26 +++++++ .../src/components/sections/NumberCounter.tsx | 14 +++- .../sections/Testimonials.toHtml.test.ts | 25 +++++++ .../src/components/sections/Testimonials.tsx | 10 ++- craft/src/utils/escape.test.ts | 55 +++++++++++++- craft/src/utils/escape.ts | 29 +++++++ 45 files changed, 1039 insertions(+), 26 deletions(-) create mode 100644 craft/src/components/basic/ButtonLink.toHtml.test.ts create mode 100644 craft/src/components/basic/Divider.toHtml.test.ts create mode 100644 craft/src/components/basic/Footer.toHtml.test.ts create mode 100644 craft/src/components/basic/Icon.toHtml.test.ts create mode 100644 craft/src/components/basic/Logo.toHtml.test.ts create mode 100644 craft/src/components/basic/Spacer.toHtml.test.ts create mode 100644 craft/src/components/basic/TextBlock.toHtml.test.ts create mode 100644 craft/src/components/forms/FormButton.toHtml.test.ts create mode 100644 craft/src/components/forms/SubscribeForm.toHtml.test.ts create mode 100644 craft/src/components/layout/BackgroundSection.toHtml.test.ts create mode 100644 craft/src/components/layout/Section.toHtml.test.ts create mode 100644 craft/src/components/media/ImageBlock.toHtml.test.ts create mode 100644 craft/src/components/sections/HeroSimple.toHtml.test.ts diff --git a/craft/src/components/basic/ButtonLink.toHtml.test.ts b/craft/src/components/basic/ButtonLink.toHtml.test.ts new file mode 100644 index 0000000..82fe9fe --- /dev/null +++ b/craft/src/components/basic/ButtonLink.toHtml.test.ts @@ -0,0 +1,54 @@ +import { describe, test, expect } from 'vitest'; +import { ButtonLink } from './ButtonLink'; + +const toHtml = (ButtonLink as any).toHtml; + +describe('ButtonLink.toHtml href sanitization (attacker-controlled `href` prop)', () => { + test('a javascript: URL is neutralized', () => { + const { html } = toHtml({ href: 'javascript:alert(1)', text: 'Click' }, ''); + expect(html).not.toContain('javascript:alert'); + }); + + test('a quote-breakout href does not escape the href attribute', () => { + const malicious = '">'; + const { html } = toHtml({ href: malicious, text: 'Click' }, ''); + expect(html).not.toContain(''); + }); + + test('a normal href still renders correctly', () => { + const { html } = toHtml({ href: 'https://example.com', text: 'Click' }, ''); + expect(html).toContain('href="https://example.com"'); + }); +}); + +describe('ButtonLink.toHtml target (boolean-gated, not raw interpolation)', () => { + test('an attribute-breakout value for target does not reach the output raw', () => { + const malicious = '_blank" onmouseover="alert(1)' as any; + const { html } = toHtml({ href: '#', text: 'x', target: malicious }, ''); + expect(html).not.toContain('onmouseover'); + }); + + test('target="_blank" still adds rel=noopener noreferrer', () => { + const { html } = toHtml({ href: '#', text: 'x', target: '_blank' }, ''); + expect(html).toContain('target="_blank"'); + expect(html).toContain('rel="noopener noreferrer"'); + }); +}); + +describe('ButtonLink.toHtml text escaping (attacker-controlled `text` prop)', () => { + test('a tag-breakout attempt in text is neutralized (no injected element)', () => { + const { html } = toHtml({ href: '#', text: '' }, ''); + expect(html).not.toContain(' { + const { html } = toHtml({ href: '#', text: 'Tom & Jerry' }, ''); + expect(html).toContain('Tom & Jerry'); + }); + + test('a normal text value still renders unchanged', () => { + const { html } = toHtml({ href: '#', text: 'Click Me' }, ''); + expect(html).toContain('>Click Me'); + }); +}); diff --git a/craft/src/components/basic/ButtonLink.tsx b/craft/src/components/basic/ButtonLink.tsx index 893485f..2b46a4f 100644 --- a/craft/src/components/basic/ButtonLink.tsx +++ b/craft/src/components/basic/ButtonLink.tsx @@ -1,7 +1,7 @@ import React, { CSSProperties } from 'react'; import { useNode, UserComponent } from '@craftjs/core'; import { cssPropsToString } from '../../utils/style-helpers'; -import { escapeAttr, safeUrl } from '../../utils/escape'; +import { escapeHtml, escapeAttr, safeUrl } from '../../utils/escape'; interface ButtonLinkProps { text?: string; @@ -78,7 +78,7 @@ ButtonLink.craft = { textDecoration: 'none', ...props.style, }); - const escapedText = (props.text || '').replace(//g, '>'); + const escapedText = escapeHtml(props.text || ''); const targetAttr = props.target === '_blank' ? ' target="_blank" rel="noopener noreferrer"' : ''; return { html: `${escapedText}`, diff --git a/craft/src/components/basic/Divider.toHtml.test.ts b/craft/src/components/basic/Divider.toHtml.test.ts new file mode 100644 index 0000000..db186b5 --- /dev/null +++ b/craft/src/components/basic/Divider.toHtml.test.ts @@ -0,0 +1,34 @@ +import { describe, test, expect } from 'vitest'; +import { Divider } from './Divider'; + +const toHtml = (Divider as any).toHtml; + +describe('Divider.toHtml normal rendering', () => { + test('renders thickness/color into the border-top style', () => { + const { html } = toHtml({ thickness: '2px', color: '#ff0000' }, ''); + expect(html).toContain('border-top:2px solid #ff0000'); + }); +}); + +describe('Divider.toHtml XSS hardening (thickness/color into style=)', () => { + test('a thickness value with an attribute-breakout string cannot escape style=""', () => { + const malicious = '1px" onmouseover="alert(1)'; + const { html } = toHtml({ thickness: malicious as any, color: '#000' }, ''); + // The quote must not survive unescaped -- otherwise it closes style="" + // early and "onmouseover" becomes a live, attacker-controlled attribute. + expect(html).not.toMatch(/"\s+onmouseover="/); + expect(html).not.toMatch(/style="[^"]*"[^>]*onmouseover/); + }); + + test('a color value with a '; + const { html } = toHtml({ thickness: '1px', color: malicious as any }, ''); + expect(html).not.toContain(''); + }); + + test('a non-string thickness (object) does not raw-splice into style=""', () => { + const malicious = { toString: () => '1px" onmouseover="alert(1)' }; + const { html } = toHtml({ thickness: malicious as any, color: '#000' }, ''); + expect(html).not.toMatch(/"\s+onmouseover="/); + }); +}); diff --git a/craft/src/components/basic/Footer.toHtml.test.ts b/craft/src/components/basic/Footer.toHtml.test.ts new file mode 100644 index 0000000..c25d19a --- /dev/null +++ b/craft/src/components/basic/Footer.toHtml.test.ts @@ -0,0 +1,22 @@ +import { describe, test, expect } from 'vitest'; +import { Footer } from './Footer'; + +const toHtml = (Footer as any).toHtml; + +describe('Footer.toHtml text escaping (attacker-controlled `text` prop)', () => { + test('a tag-breakout attempt in text is neutralized (no injected element)', () => { + const { html } = toHtml({ text: '' }, ''); + expect(html).not.toContain(' { + const { html } = toHtml({ text: 'Terms & Conditions' }, ''); + expect(html).toContain('Terms & Conditions'); + }); + + test('a normal copyright text value still renders unchanged', () => { + const { html } = toHtml({ text: '© 2026 MySite. All rights reserved.' }, ''); + expect(html).toContain('© 2026 MySite. All rights reserved.'); + }); +}); diff --git a/craft/src/components/basic/Footer.tsx b/craft/src/components/basic/Footer.tsx index e63dbf7..f8b87d0 100644 --- a/craft/src/components/basic/Footer.tsx +++ b/craft/src/components/basic/Footer.tsx @@ -1,6 +1,7 @@ import React, { CSSProperties, useCallback, useRef, useEffect } from 'react'; import { useNode, UserComponent } from '@craftjs/core'; import { cssPropsToString } from '../../utils/style-helpers'; +import { escapeHtml } from '../../utils/escape'; interface FooterProps { text?: string; @@ -107,6 +108,6 @@ Footer.craft = { textAlign: 'center', ...props.style, }); - const escapedText = (props.text || '').replace(//g, '>'); + const escapedText = escapeHtml(props.text || ''); return { html: `${escapedText}` }; }; diff --git a/craft/src/components/basic/Heading.toHtml.test.ts b/craft/src/components/basic/Heading.toHtml.test.ts index f13d115..112481f 100644 --- a/craft/src/components/basic/Heading.toHtml.test.ts +++ b/craft/src/components/basic/Heading.toHtml.test.ts @@ -38,3 +38,21 @@ describe('Heading.toHtml level allowlist (adversarial re-review, same class as C } }); }); + +describe('Heading.toHtml text escaping (attacker-controlled `text` prop)', () => { + test('a tag-breakout attempt in text is neutralized (no injected element)', () => { + const { html } = toHtml({ text: '', level: 'h2' }, ''); + expect(html).not.toContain(' { + const { html } = toHtml({ text: 'Fish & Chips', level: 'h2' }, ''); + expect(html).toContain('Fish & Chips'); + }); + + test('a normal text value still renders unchanged', () => { + const { html } = toHtml({ text: 'Hello world', level: 'h2' }, ''); + expect(html).toBe('

Hello world

'); + }); +}); diff --git a/craft/src/components/basic/Heading.tsx b/craft/src/components/basic/Heading.tsx index da3e654..4f0accd 100644 --- a/craft/src/components/basic/Heading.tsx +++ b/craft/src/components/basic/Heading.tsx @@ -1,6 +1,7 @@ import React, { CSSProperties, useCallback, useRef, useEffect } from 'react'; import { useNode, UserComponent } from '@craftjs/core'; import { cssPropsToString } from '../../utils/style-helpers'; +import { escapeHtml } from '../../utils/escape'; type HeadingLevel = 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'; @@ -110,7 +111,7 @@ Heading.craft = { (Heading as any).toHtml = (props: HeadingProps, _childrenHtml: string) => { const tag = sanitizeHeadingLevel(props.level); - const safeText = (props.text || '').replace(//g, '>'); + const safeText = escapeHtml(props.text || ''); const styleStr = cssPropsToString(props.style); return { html: `<${tag}${styleStr ? ` style="${styleStr}"` : ''}>${safeText}` }; }; diff --git a/craft/src/components/basic/HtmlBlock.toHtml.test.ts b/craft/src/components/basic/HtmlBlock.toHtml.test.ts index d209ad5..35e939e 100644 --- a/craft/src/components/basic/HtmlBlock.toHtml.test.ts +++ b/craft/src/components/basic/HtmlBlock.toHtml.test.ts @@ -10,4 +10,16 @@ describe('HtmlBlock.toHtml sanitizes raw code (A4.1)', () => { expect(html).not.toContain('onclick'); expect(html).toContain('

hi

'); }); + + test('does not wrap output in an unsanitized element carrying the style prop raw', () => { + // toHtml only ever returns the sanitized `code` blob -- there is no + // wrapper
in the exported HTML, so a malicious + // `style` prop (e.g. an attacker-controlled object with a breakout + // toString()) has nothing to splice into. + const malicious = { toString: () => 'color:red" onmouseover="alert(1)' } as any; + const { html } = toHtml({ code: '

hi

', style: malicious }, ''); + expect(html).not.toMatch(/onmouseover/); + expect(html).not.toMatch(/
hi

'); + }); }); diff --git a/craft/src/components/basic/Icon.toHtml.test.ts b/craft/src/components/basic/Icon.toHtml.test.ts new file mode 100644 index 0000000..cac27f9 --- /dev/null +++ b/craft/src/components/basic/Icon.toHtml.test.ts @@ -0,0 +1,46 @@ +import { describe, test, expect } from 'vitest'; +import { Icon } from './Icon'; + +const toHtml = (Icon as any).toHtml; + +describe('Icon.toHtml normal rendering', () => { + test('renders icon class, size/color style, and link href', () => { + const { html } = toHtml({ icon: 'fa-star', size: '32px', color: '#3b82f6', link: 'https://example.com' }, ''); + expect(html).toContain('class="fa fa-star"'); + expect(html).toContain('font-size:32px'); + expect(html).toContain('color:#3b82f6'); + expect(html).toContain('href="https://example.com"'); + }); +}); + +describe('Icon.toHtml XSS hardening', () => { + test('an icon name with an attribute-breakout string is escaped, not raw-concatenated', () => { + const malicious = 'star">'; + const { html } = toHtml({ icon: malicious as any }, ''); + expect(html).not.toContain(''); + expect(html).not.toMatch(/class="fa star">'; + const { html } = toHtml({ href: malicious }, ''); + expect(html).not.toContain(''); + }); +}); + +describe('Logo.toHtml image src/alt sanitization (type="image")', () => { + test('a javascript: imageSrc is neutralized', () => { + const { html } = toHtml({ type: 'image', imageSrc: 'javascript:alert(1)', text: 'Logo' }, ''); + expect(html).not.toContain('javascript:alert'); + }); + + test('a quote-breakout alt (from `text`) does not escape the img attribute', () => { + const malicious = '">'; + const { html } = toHtml({ type: 'image', imageSrc: 'https://example.com/logo.png', text: malicious }, ''); + expect(html).not.toContain(''); + }); + + test('a non-numeric imageWidth (attribute-breakout attempt) does not escape the style attribute', () => { + const malicious = '1">'; + const { html } = toHtml({ type: 'image', imageSrc: 'https://example.com/logo.png', imageWidth: malicious }, ''); + expect(html).not.toContain(''); + }); +}); + +describe('Logo.toHtml text-logo styling sanitization', () => { + test('a quote-breakout color does not escape the span style attribute', () => { + const malicious = 'red" onmouseover="alert(1)'; + const { html } = toHtml({ type: 'text', text: 'MySite', color: malicious }, ''); + // The raw `"` must never survive un-escaped inside the style attribute + // value -- if it did, `onmouseover` would land as a REAL new HTML + // attribute (breakout) rather than being inert CSS-value garbage inside + // a properly-escaped style="...". + expect(html).not.toMatch(/style="[^"]*"[^>]*onmouseover/); + }); + + test('a normal logo renders as expected', () => { + const { html } = toHtml({ type: 'text', text: 'MySite', href: '/' }, ''); + expect(html).toContain('href="/"'); + expect(html).toContain('MySite'); + }); +}); diff --git a/craft/src/components/basic/SearchBar.toHtml.test.ts b/craft/src/components/basic/SearchBar.toHtml.test.ts index acf29a1..9b93acc 100644 --- a/craft/src/components/basic/SearchBar.toHtml.test.ts +++ b/craft/src/components/basic/SearchBar.toHtml.test.ts @@ -11,3 +11,22 @@ describe('SearchBar.toHtml decorative icons (F2.5)', () => { icons.forEach((tag: string) => expect(tag).toContain('aria-hidden="true"')); }); }); + +describe('SearchBar.toHtml XSS hardening (placeholder/buttonText/showButton)', () => { + test('a placeholder value with an attribute-breakout string cannot escape placeholder=""', () => { + const malicious = 'Search..." onmouseover="alert(1)'; + const { html } = toHtml({ placeholder: malicious }, ''); + expect(html).not.toMatch(/"\s+onmouseover="/); + }); + + test('a buttonText value with a script tag is escaped as text content, not raw HTML', () => { + const malicious = ''; + const { html } = toHtml({ buttonText: malicious, showButton: true }, ''); + expect(html).not.toContain(''); + }); + + test('a non-boolean showButton (string "false") still yields fixed, safe border-radius values', () => { + const { html } = toHtml({ showButton: 'false' as any }, ''); + expect(html).toMatch(/border-radius:(8px 0 0 8px|8px)/); + }); +}); diff --git a/craft/src/components/basic/SocialLinks.toHtml.test.ts b/craft/src/components/basic/SocialLinks.toHtml.test.ts index a415453..85aa97a 100644 --- a/craft/src/components/basic/SocialLinks.toHtml.test.ts +++ b/craft/src/components/basic/SocialLinks.toHtml.test.ts @@ -14,3 +14,40 @@ describe('SocialLinks.toHtml accessibility (F2.5)', () => { expect(html).toMatch(/]*aria-hidden="true"/); }); }); + +describe('SocialLinks.toHtml XSS hardening (iconSize/iconColor/iconBgColor/gap into style=)', () => { + test('an iconSize value with an attribute-breakout string cannot escape style=""', () => { + const malicious = '20px" onmouseover="alert(1)'; + const { html } = toHtml({ links: [{ platform: 'facebook', url: '#' }], iconSize: malicious as any }, ''); + expect(html).not.toMatch(/"\s+onmouseover="/); + }); + + test('an iconColor value with an attribute-breakout string cannot escape style=""', () => { + const malicious = '#fff" onmouseover="alert(1)'; + const { html } = toHtml({ links: [{ platform: 'facebook', url: '#' }], iconColor: malicious as any }, ''); + expect(html).not.toMatch(/"\s+onmouseover="/); + }); + + test('an iconBgColor value with an attribute-breakout string cannot escape style=""', () => { + const malicious = '#374151" onmouseover="alert(1)'; + const { html } = toHtml({ links: [{ platform: 'facebook', url: '#' }], iconShape: 'circle', iconBgColor: malicious as any }, ''); + expect(html).not.toMatch(/"\s+onmouseover="/); + }); + + test('a gap value with an attribute-breakout string cannot escape the wrapper style=""', () => { + const malicious = '10px" onmouseover="alert(1)'; + const { html } = toHtml({ links: [{ platform: 'facebook', url: '#' }], gap: malicious as any }, ''); + expect(html).not.toMatch(/"\s+onmouseover="/); + }); + + test('a malicious platform key does not produce a raw class-attribute breakout', () => { + const malicious = 'x">'; + const { html } = toHtml({ links: [{ platform: malicious, url: '#' }] }, ''); + expect(html).not.toContain(''); + }); + + test('a link url with a javascript: scheme is neutralized', () => { + const { html } = toHtml({ links: [{ platform: 'facebook', url: 'javascript:alert(1)' }] }, ''); + expect(html).not.toContain('javascript:alert(1)'); + }); +}); diff --git a/craft/src/components/basic/Spacer.toHtml.test.ts b/craft/src/components/basic/Spacer.toHtml.test.ts new file mode 100644 index 0000000..5904756 --- /dev/null +++ b/craft/src/components/basic/Spacer.toHtml.test.ts @@ -0,0 +1,26 @@ +import { describe, test, expect } from 'vitest'; +import { Spacer } from './Spacer'; + +const toHtml = (Spacer as any).toHtml; + +describe('Spacer.toHtml normal rendering', () => { + test('renders height into the style attribute', () => { + const { html } = toHtml({ height: '80px' }, ''); + expect(html).toContain('height:80px'); + }); +}); + +describe('Spacer.toHtml XSS hardening (height into style=)', () => { + test('a height value with an attribute-breakout string cannot escape style=""', () => { + const malicious = '40px" onmouseover="alert(1)'; + const { html } = toHtml({ height: malicious as any }, ''); + expect(html).not.toMatch(/"\s+onmouseover="/); + expect(html).not.toMatch(/style="[^"]*"[^>]*onmouseover/); + }); + + test('a height value with a '; + const { html } = toHtml({ height: malicious as any }, ''); + expect(html).not.toContain(''); + }); +}); diff --git a/craft/src/components/basic/StarRating.toHtml.test.ts b/craft/src/components/basic/StarRating.toHtml.test.ts index 0d7640c..632259f 100644 --- a/craft/src/components/basic/StarRating.toHtml.test.ts +++ b/craft/src/components/basic/StarRating.toHtml.test.ts @@ -40,3 +40,38 @@ describe('StarRating.toHtml XSS hardening (filledColor/emptyColor/size into styl expect(html).toContain('color:#ff9900'); }); }); + +describe('StarRating.toHtml XSS hardening (rating/maxStars into aria-label, F2.2 CONFIRMED sink)', () => { + test('a maxStars value with an attribute-breakout string is neutralized in aria-label', () => { + const malicious = '5" onmouseover="alert(1)'; + const { html } = toHtml({ rating: 3, maxStars: malicious as any }, ''); + expect(html).not.toMatch(/onmouseover/); + expect(html).not.toMatch(/aria-label="Rating: 3 out of 5" onmouseover/); + }); + + test('a rating value with an attribute-breakout string is neutralized in aria-label', () => { + const malicious = '4.5" onmouseover="alert(1)'; + const { html } = toHtml({ rating: malicious as any, maxStars: 5 }, ''); + expect(html).not.toMatch(/onmouseover/); + }); + + test('a non-numeric maxStars does not blow up the star loop (no NaN glyph count, no huge output)', () => { + const malicious = '5" onmouseover="alert(1)'; + const { html } = toHtml({ rating: 3, maxStars: malicious as any }, ''); + const glyphs = html.match(/ { + const { html } = toHtml({ rating: 3, maxStars: 1e9 as any }, ''); + const glyphs = html.match(/ { + const { html } = toHtml({ rating: 4.5, maxStars: 5 }, ''); + expect(html).toMatch(/ { - const rating = props.rating ?? 4.5; - const maxStars = props.maxStars || 5; + // `rating`/`maxStars` are declared `number` in TS but arrive unchecked at + // runtime (AI update_props only validates node_id; deserialized saved + // state is untyped JSON) -- a string like `5" onmouseover="alert(1)` + // breaks out of the aria-label attribute below, and an uncoerced/unclamped + // maxStars can also blow up the star-glyph loop (NaN, absurd loop count, + // or -- observed -- a RangeError from string concatenation overflow with + // e.g. maxStars=1e9). Coerce to numbers with sane fallbacks/clamps first. + const ratingRaw = Number(props.rating); + const rating = Number.isFinite(ratingRaw) ? ratingRaw : 4.5; + const maxStarsRaw = Number(props.maxStars); + const maxStars = Number.isFinite(maxStarsRaw) + ? Math.min(Math.max(Math.trunc(maxStarsRaw), 0), 50) + : 5; // Sanitized -- raw string-interpolation sinks in the star glyphs below. const size = cssValue(props.size) || '24px'; const filledColor = cssValue(props.filledColor) || '#f59e0b'; @@ -125,7 +136,12 @@ StarRating.craft = { // The star glyphs convey nothing to assistive tech on their own -- wrap // in role="img" with a textual equivalent, and hide the decorative glyphs // themselves (aria-hidden above) so AT doesn't announce each icon. + // Belt-and-suspenders: rating/maxStars are already coerced to numbers + // above, but the assembled label is still run through escapeAttr() in + // case a decimal/negative/Infinity edge case produces odd (though no + // longer dangerous) text. + const ariaLabel = escapeAttr(`Rating: ${rating} out of ${maxStars}`); return { - html: `${starsHtml}`, + html: `${starsHtml}`, }; }; diff --git a/craft/src/components/basic/TextBlock.toHtml.test.ts b/craft/src/components/basic/TextBlock.toHtml.test.ts new file mode 100644 index 0000000..433ff72 --- /dev/null +++ b/craft/src/components/basic/TextBlock.toHtml.test.ts @@ -0,0 +1,22 @@ +import { describe, test, expect } from 'vitest'; +import { TextBlock } from './TextBlock'; + +const toHtml = (TextBlock as any).toHtml; + +describe('TextBlock.toHtml text escaping (attacker-controlled `text` prop)', () => { + test('a tag-breakout attempt in text is neutralized (no injected element)', () => { + const { html } = toHtml({ text: '

' }, ''); + expect(html).not.toContain(' { + const { html } = toHtml({ text: 'Tom & Jerry' }, ''); + expect(html).toContain('Tom & Jerry'); + }); + + test('a normal text value still renders unchanged', () => { + const { html } = toHtml({ text: 'Hello world' }, ''); + expect(html).toBe('

Hello world

'); + }); +}); diff --git a/craft/src/components/basic/TextBlock.tsx b/craft/src/components/basic/TextBlock.tsx index 5cb19f7..f6c6d26 100644 --- a/craft/src/components/basic/TextBlock.tsx +++ b/craft/src/components/basic/TextBlock.tsx @@ -1,6 +1,7 @@ import React, { CSSProperties, useCallback, useRef, useEffect } from 'react'; import { useNode, UserComponent } from '@craftjs/core'; import { cssPropsToString } from '../../utils/style-helpers'; +import { escapeHtml } from '../../utils/escape'; interface TextBlockProps { text?: string; @@ -95,6 +96,6 @@ TextBlock.craft = { (TextBlock as any).toHtml = (props: TextBlockProps, _childrenHtml: string) => { const styleStr = cssPropsToString(props.style); - const escapedText = (props.text || '').replace(//g, '>'); + const escapedText = escapeHtml(props.text || ''); return { html: `${escapedText}

` }; }; diff --git a/craft/src/components/forms/ContactForm.toHtml.test.ts b/craft/src/components/forms/ContactForm.toHtml.test.ts index 4f50fec..42cd974 100644 --- a/craft/src/components/forms/ContactForm.toHtml.test.ts +++ b/craft/src/components/forms/ContactForm.toHtml.test.ts @@ -110,3 +110,19 @@ describe('ContactForm.toHtml accessibility (F2.1)', () => { expect(ids1).toEqual(ids2); }); }); + +describe('ContactForm.toHtml field type attribute sanitization', () => { + test('malicious field.type cannot break out of the input attribute; falls back to type="text"', () => { + const fields = [{ type: 'text">' as any, label: 'Name', name: 'name', placeholder: 'Your name', required: false }]; + const { html } = toHtml({ fields }, ''); + expect(html).not.toContain(' { + const fields = [{ type: 'email' as const, label: 'Email', name: 'email', placeholder: 'you@example.com', required: false }]; + const { html } = toHtml({ fields }, ''); + expect(html).toContain('type="email"'); + }); +}); diff --git a/craft/src/components/forms/ContactForm.tsx b/craft/src/components/forms/ContactForm.tsx index c86fdae..3493b16 100644 --- a/craft/src/components/forms/ContactForm.tsx +++ b/craft/src/components/forms/ContactForm.tsx @@ -2,7 +2,7 @@ import React, { CSSProperties } from 'react'; import { useNode, UserComponent } from '@craftjs/core'; import { cssPropsToString } from '../../utils/style-helpers'; import { relayFormWiring } from '../../utils/form-relay-wiring'; -import { escapeHtml, escapeAttr, slugId, cssValue } from '../../utils/escape'; +import { escapeHtml, escapeAttr, slugId, cssValue, sanitizeInputType } from '../../utils/escape'; interface ContactFormField { type: 'text' | 'email' | 'tel' | 'textarea' | 'select'; @@ -196,7 +196,7 @@ ContactForm.craft = { const opts = (field.options || []).map((o) => ``).join(''); inputHtml = ``; } else { - inputHtml = ``; + inputHtml = ``; } return `
${labelHtml}${inputHtml}
`; }).join('\n '); diff --git a/craft/src/components/forms/FormButton.toHtml.test.ts b/craft/src/components/forms/FormButton.toHtml.test.ts new file mode 100644 index 0000000..d8b879d --- /dev/null +++ b/craft/src/components/forms/FormButton.toHtml.test.ts @@ -0,0 +1,25 @@ +import { describe, test, expect } from 'vitest'; +import { FormButton } from './FormButton'; + +const toHtml = (FormButton as any).toHtml; + +describe('FormButton.toHtml', () => { + test('normal text renders as-is', () => { + const { html } = toHtml({ text: 'Send it' }, ''); + expect(html).toContain('>Send it<'); + expect(html).toContain('type="submit"'); + }); + + test('type="submit" is a hardcoded literal, not prop-driven', () => { + const { html } = toHtml({ text: 'Submit' }, ''); + expect(html).toMatch(/
`, }; }; diff --git a/craft/src/components/forms/SubscribeForm.toHtml.test.ts b/craft/src/components/forms/SubscribeForm.toHtml.test.ts new file mode 100644 index 0000000..39600dd --- /dev/null +++ b/craft/src/components/forms/SubscribeForm.toHtml.test.ts @@ -0,0 +1,39 @@ +import { describe, test, expect } from 'vitest'; +import { SubscribeForm } from './SubscribeForm'; + +const toHtml = (SubscribeForm as any).toHtml; + +describe('SubscribeForm.toHtml hardcoded attributes stay hardcoded (no raw prop breakout)', () => { + test('form method is always POST regardless of any injected props', () => { + const { html } = toHtml({ heading: 'Join us', method: 'GET">' } as any, ''); + expect(html).toContain('
{ + const { html } = toHtml({ type: 'text">' } as any, ''); + expect(html).toContain(' { + const { html: inlineHtml } = toHtml({ layout: 'inline' }, ''); + const { html: stackedHtml } = toHtml({ layout: 'stacked' }, ''); + expect(inlineHtml).toContain('flex-direction:row'); + expect(stackedHtml).toContain('flex-direction:column'); + }); + + test('malicious layout value cannot inject raw CSS/attribute breakout (falls through the isInline boolean check to the stacked literal)', () => { + const { html } = toHtml({ layout: '">' as any }, ''); + expect(html).not.toContain(' { + const { html } = toHtml({ heading: 'Subscribe', placeholder: 'you@example.com', buttonText: 'Go' }, ''); + expect(html).toContain('Subscribe'); + expect(html).toContain('placeholder="you@example.com"'); + expect(html).toContain('>Go<'); + }); +}); diff --git a/craft/src/components/forms/TextareaField.toHtml.test.ts b/craft/src/components/forms/TextareaField.toHtml.test.ts index b538aa0..e5227a1 100644 --- a/craft/src/components/forms/TextareaField.toHtml.test.ts +++ b/craft/src/components/forms/TextareaField.toHtml.test.ts @@ -50,3 +50,16 @@ describe('TextareaField.toHtml deterministic + unique ids (thread node id, resol expect(id1).toBe(id2); }); }); + +describe('TextareaField.toHtml rows attribute sanitization', () => { + test('malicious rows value cannot break out of the attribute; falls back to a numeric rows', () => { + const { html } = toHtml({ label: 'Message', name: 'message', rows: '4">' as any }, ''); + expect(html).not.toContain(' { + const { html } = toHtml({ label: 'Message', name: 'message', rows: 8 }, ''); + expect(html).toContain('rows="8"'); + }); +}); diff --git a/craft/src/components/forms/TextareaField.tsx b/craft/src/components/forms/TextareaField.tsx index 0d2ff6c..526a4a9 100644 --- a/craft/src/components/forms/TextareaField.tsx +++ b/craft/src/components/forms/TextareaField.tsx @@ -97,6 +97,11 @@ TextareaField.craft = { ...props.style, }); const reqAttr = props.required ? ' required' : ''; + // `rows` is declared as a TS `number` but arrives unchecked (AI update_props + // path only validates node_id; deserialized saved-state JSON is untyped at + // runtime), so a string like `4">'); + expect(html).not.toContain('javascript:alert(1)'); + }); + + test('a malicious bgColor cannot break out of the outer style attribute', () => { + const malicious = 'red" onmouseover="alert(1)'; + const { html } = toHtml({ bgColor: malicious }, 'child'); + expect(html).not.toContain('onmouseover="alert(1)"'); + }); + + test('a malicious overlayColor cannot break out of the overlay style attribute', () => { + const malicious = 'red" onmouseover="alert(1)'; + const { html } = toHtml({ overlayColor: malicious }, 'child'); + expect(html).not.toContain('onmouseover="alert(1)"'); + }); + + test('a wrong-typed overlayOpacity (string, not number) cannot break out of the overlay style attribute', () => { + const malicious = '0.4" onmouseover="alert(1)' as any; + const { html } = toHtml({ overlayOpacity: malicious }, 'child'); + expect(html).not.toContain('onmouseover="alert(1)"'); + }); + + test('a malicious innerMaxWidth cannot break out of the inner style attribute', () => { + const malicious = '1200px" onmouseover="alert(1)'; + const { html } = toHtml({ innerMaxWidth: malicious }, 'child'); + expect(html).not.toContain('onmouseover="alert(1)"'); + }); + + test('normal props still render correctly', () => { + const { html } = toHtml({ bgImage: 'https://example.com/bg.jpg', bgColor: '#1e293b', overlayColor: '#000000', overlayOpacity: 0.4, innerMaxWidth: '1200px' }, 'child'); + expect(html).toContain("url('https://example.com/bg.jpg')"); + expect(html).toContain('background-color:#1e293b'); + expect(html).toContain('opacity:0.4'); + expect(html).toContain('max-width:1200px'); + expect(html).toContain('child'); + }); +}); diff --git a/craft/src/components/layout/Section.toHtml.test.ts b/craft/src/components/layout/Section.toHtml.test.ts new file mode 100644 index 0000000..316661d --- /dev/null +++ b/craft/src/components/layout/Section.toHtml.test.ts @@ -0,0 +1,75 @@ +import { describe, test, expect } from 'vitest'; +import { Section } from './Section'; + +const toHtml = (Section as any).toHtml; + +describe('Section.toHtml anchorId', () => { + test('escapes a malicious anchorId (attribute breakout attempt)', () => { + const { html } = toHtml({ anchorId: 'x" onmouseover="alert(1)' }, 'child'); + expect(html).not.toContain('onmouseover="alert(1)"'); + }); + + test('a normal anchorId still renders correctly', () => { + const { html } = toHtml({ anchorId: 'my-section' }, 'child'); + expect(html).toContain('id="my-section"'); + }); +}); + +describe('Section.toHtml childrenHtml passthrough', () => { + test('children are preserved', () => { + const { html } = toHtml({}, '

hello

'); + expect(html).toContain('

hello

'); + }); +}); + +describe('Section.toHtml shape divider color/height XSS hardening', () => { + test('a malicious topDividerColor cannot break out of the SVG style attribute', () => { + const malicious = 'red" onmouseover="alert(1)'; + const { html } = toHtml({ topDivider: 'wave', topDividerColor: malicious }, ''); + expect(html).not.toContain('onmouseover="alert(1)"'); + }); + + test('a malicious topDividerColor cannot inject a '; + const { html } = toHtml({ topDivider: 'wave', topDividerColor: malicious }, ''); + expect(html).not.toContain(''); + }); + + test('a malicious bottomDividerHeight cannot break out of the wrapper style attribute', () => { + const malicious = '50px" onmouseover="alert(1)'; + const { html } = toHtml({ bottomDivider: 'angle', bottomDividerHeight: malicious }, ''); + expect(html).not.toContain('onmouseover="alert(1)"'); + }); + + test('a normal divider color/height still renders correctly', () => { + const { html } = toHtml({ topDivider: 'wave', topDividerColor: '#123456', topDividerHeight: '80px' }, ''); + expect(html).toContain('fill:#123456'); + expect(html).toContain('height:80px'); + }); + + test('divider shape "none" emits no divider markup', () => { + const { html } = toHtml({ topDivider: 'none' }, 'child'); + expect(html).not.toContain(' { + const malicious = 'wave">' as any; + const { html } = toHtml({ topDivider: malicious }, 'child'); + expect(html).not.toContain(''); + expect(html).not.toContain(' { + const { html } = toHtml({ topDivider: '__proto__' as any }, 'child'); + expect(html).not.toContain('[object'); + expect(html).not.toContain('native code'); + expect(html).not.toContain(' { + const { html } = toHtml({ topDivider: 'toString' as any }, 'child'); + expect(html).not.toContain('[object'); + expect(html).not.toContain('native code'); + expect(html).not.toContain(' = ({ shape, color, height, position }) => { if (!shape || shape === 'none') return null; - const path = DIVIDER_PATHS[shape]; + // `shape` is attacker-controlled (AI update_props / deserialized state) and + // not runtime-type-checked. A plain-object index lookup with a string key + // like '__proto__', 'toString', or 'constructor' returns an INHERITED + // Object.prototype value (not undefined), which would otherwise leak + // "[object Object]" / a function's source text into the SVG `d` attribute + // below. hasOwnProperty restricts the lookup to the real allowlisted keys. + if (!Object.prototype.hasOwnProperty.call(DIVIDER_PATHS, shape)) return null; + const path = DIVIDER_PATHS[shape as Exclude]; if (!path) return null; const isTop = position === 'top'; @@ -162,7 +169,10 @@ function buildDividerHtml( position: 'top' | 'bottom', ): string { if (!shape || shape === 'none') return ''; - const path = DIVIDER_PATHS[shape]; + // See the matching hasOwnProperty guard in above -- same + // prototype-pollution-shaped lookup, same fix. + if (!Object.prototype.hasOwnProperty.call(DIVIDER_PATHS, shape)) return ''; + const path = DIVIDER_PATHS[shape as Exclude]; if (!path) return ''; const isTop = position === 'top'; diff --git a/craft/src/components/media/ImageBlock.toHtml.test.ts b/craft/src/components/media/ImageBlock.toHtml.test.ts new file mode 100644 index 0000000..8729db2 --- /dev/null +++ b/craft/src/components/media/ImageBlock.toHtml.test.ts @@ -0,0 +1,34 @@ +import { describe, test, expect } from 'vitest'; +import { ImageBlock } from './ImageBlock'; + +const toHtml = (ImageBlock as any).toHtml; + +describe('ImageBlock.toHtml src/alt XSS hardening', () => { + test('a javascript: src never reaches the output', () => { + const { html } = toHtml({ src: 'javascript:alert(1)' }, ''); + expect(html).not.toContain('javascript:'); + }); + + test('a malicious src cannot break out of the src attribute', () => { + const malicious = 'https://example.com/x.jpg" onerror="alert(1)'; + const { html } = toHtml({ src: malicious }, ''); + expect(html).not.toContain('onerror="alert(1)"'); + }); + + test('a malicious alt cannot break out of the alt attribute', () => { + const malicious = 'x" onerror="alert(1)'; + const { html } = toHtml({ src: 'https://example.com/x.jpg', alt: malicious }, ''); + expect(html).not.toContain('onerror="alert(1)"'); + }); + + test('a placeholder/empty src emits no output', () => { + const { html } = toHtml({ src: '' }, ''); + expect(html).toBe(''); + }); + + test('a normal image still renders correctly', () => { + const { html } = toHtml({ src: 'https://example.com/photo.jpg', alt: 'A photo' }, ''); + expect(html).toContain('src="https://example.com/photo.jpg"'); + expect(html).toContain('alt="A photo"'); + }); +}); diff --git a/craft/src/components/media/MapEmbed.toHtml.test.ts b/craft/src/components/media/MapEmbed.toHtml.test.ts index 4143836..33482d7 100644 --- a/craft/src/components/media/MapEmbed.toHtml.test.ts +++ b/craft/src/components/media/MapEmbed.toHtml.test.ts @@ -27,3 +27,41 @@ describe('MapEmbed.toHtml iframe src ampersand encoding (F-export review Minor)' expect(srcMatch![1]).not.toMatch(/&(?!amp;)/); }); }); + +describe('MapEmbed.toHtml address/zoom/height XSS hardening', () => { + test('a malicious address cannot break out of the src or title attribute', () => { + const malicious = 'X" onerror="alert(1)'; + const { html } = toHtml({ address: malicious }, ''); + expect(html).not.toContain('onerror="alert(1)"'); + }); + + test('a wrong-typed zoom (string with attribute-breakout chars) cannot break out of the src attribute', () => { + const malicious = '14">' as any; + const { html } = toHtml({ address: 'X', zoom: malicious }, ''); + expect(html).not.toContain(''); + expect(html).not.toContain('"> { + const malicious = '14">' as any; + const { html } = toHtml({ address: 'X', zoom: malicious }, ''); + const srcMatch = html.match(/