Skip to content

Commit 58d3d3a

Browse files
yusukebeusualoma
andauthored
Merge commit from fork
* fix(jsx): prevent CSS injection via `;` in style object values * apply the patch Co-authored-by: Taku Amano <taku@taaas.jp> --------- Co-authored-by: Taku Amano <taku@taaas.jp>
1 parent 568c2ec commit 58d3d3a

4 files changed

Lines changed: 388 additions & 13 deletions

File tree

src/jsx/index.test.tsx

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,110 @@ describe('render to string', () => {
519519
const template = <h1 style={{ '--myVar': 1 }}>Hello</h1>
520520
expect(template.toString()).toBe('<h1 style="--myVar:1px">Hello</h1>')
521521
})
522+
523+
describe('CSS injection prevention', () => {
524+
it('should drop style values containing ";" to prevent CSS injection', () => {
525+
const userInput =
526+
'transparent;background:url(https://attacker.example/a.png);position:fixed;top:0'
527+
const template = <div style={{ color: userInput }} />
528+
const out = template.toString() as string
529+
expect(out).not.toContain('background:url')
530+
expect(out).not.toContain('position:fixed')
531+
expect(out).not.toContain('color:')
532+
expect(out).toBe('<div style=""></div>')
533+
})
534+
535+
it('should drop only the unsafe property and keep other safe properties', () => {
536+
const template = <div style={{ color: 'red;background:blue', backgroundColor: 'white' }} />
537+
const out = template.toString() as string
538+
expect(out).toBe('<div style="background-color:white"></div>')
539+
expect(out).not.toContain('background:blue')
540+
})
541+
542+
it('should drop style values that try to hide declaration separators in CSS comments', () => {
543+
const template = (
544+
<div
545+
style={{
546+
color: 'red/*(*/;background:blue;position:fixed;top:0',
547+
backgroundColor: 'white',
548+
}}
549+
/>
550+
)
551+
const out = template.toString() as string
552+
expect(out).toBe('<div style="background-color:white"></div>')
553+
expect(out).not.toContain('position:fixed')
554+
})
555+
556+
it('should drop style values that try to expose declarations through CSS curly blocks', () => {
557+
const template = (
558+
<div
559+
style={{
560+
color: 'red{;background:blue}',
561+
backgroundColor: 'white',
562+
}}
563+
/>
564+
)
565+
const out = template.toString() as string
566+
expect(out).toBe('<div style="background-color:white"></div>')
567+
expect(out).not.toContain('background:blue')
568+
})
569+
570+
it('should drop unterminated style values that can swallow following declarations', () => {
571+
const template = <div style={{ color: 'red/*', display: 'none' }} />
572+
const out = template.toString() as string
573+
expect(out).toBe('<div style="display:none"></div>')
574+
expect(out).not.toContain('color:')
575+
})
576+
577+
it('should drop style property names that can inject declarations', () => {
578+
const template = (
579+
<div
580+
style={{
581+
'color;background-image': 'url(https://attacker.example/a.png)',
582+
backgroundColor: 'white',
583+
}}
584+
/>
585+
)
586+
const out = template.toString() as string
587+
expect(out).toBe('<div style="background-color:white"></div>')
588+
expect(out).not.toContain('background-image')
589+
})
590+
591+
it('should still HTML-escape safe style values', () => {
592+
const template = <h1 style={{ fontFamily: '"DejaVu Sans Mono", monospace' }}>Hello</h1>
593+
expect(template.toString()).toBe(
594+
'<h1 style="font-family:&quot;DejaVu Sans Mono&quot;, monospace">Hello</h1>'
595+
)
596+
})
597+
598+
it('should keep semicolons inside quoted style values', () => {
599+
const template = <h1 style={{ fontFamily: '"a;b", sans-serif' }}>Hello</h1>
600+
expect(template.toString()).toBe(
601+
'<h1 style="font-family:&quot;a;b&quot;, sans-serif">Hello</h1>'
602+
)
603+
})
604+
605+
it('should drop quoted style values that use newlines to expose declaration separators', () => {
606+
const template = (
607+
<div
608+
style={{
609+
fontFamily: '"\n;background:url(https://attacker.example/a.png)',
610+
backgroundColor: 'white',
611+
}}
612+
/>
613+
)
614+
expect(template.toString()).toBe('<div style="background-color:white"></div>')
615+
})
616+
617+
it('should keep numeric style values working', () => {
618+
const template = <div style={{ fontSize: 10, lineHeight: 1.5 }} />
619+
expect(template.toString()).toBe('<div style="font-size:10px;line-height:1.5"></div>')
620+
})
621+
622+
it('should not throw when style values contain ";"', () => {
623+
expect(() => (<div style={{ color: 'red;evil:1' }} />).toString()).not.toThrow()
624+
})
625+
})
522626
})
523627

524628
describe('HtmlEscaped in props', () => {

src/jsx/jsx-runtime.test.tsx

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,34 @@ describe('jsx-runtime', () => {
4343
)
4444
})
4545

46+
it('Should drop style values containing ";" in jsxAttr() to prevent CSS injection', () => {
47+
expect(
48+
String(jsxAttr('style', { color: 'red;background:blue', backgroundColor: 'white' }))
49+
).toBe('style="background-color:white"')
50+
})
51+
52+
it('Should drop style values that hide declaration separators in CSS comments in jsxAttr()', () => {
53+
expect(
54+
String(
55+
jsxAttr('style', {
56+
color: 'red/*(*/;background:blue;position:fixed;top:0',
57+
backgroundColor: 'white',
58+
})
59+
)
60+
).toBe('style="background-color:white"')
61+
})
62+
63+
it('Should drop style property names that can inject declarations in jsxAttr()', () => {
64+
expect(
65+
String(
66+
jsxAttr('style', {
67+
'color;background-image': 'url(https://attacker.example/a.png)',
68+
backgroundColor: 'white',
69+
})
70+
)
71+
).toBe('style="background-color:white"')
72+
})
73+
4674
// https://en.reactjs.org/docs/jsx-in-depth.html#booleans-null-and-undefined-are-ignored
4775
describe('Booleans, Null, and Undefined Are Ignored', () => {
4876
it.each([true, false, undefined, null])('%s', (item) => {

src/jsx/utils.test.ts

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,4 +214,150 @@ describe('styleObjectForEach', () => {
214214
)
215215
})
216216
})
217+
218+
describe('Should skip values containing ";" to prevent CSS injection', () => {
219+
it('skips a value with ";"', () => {
220+
const fn = vi.fn()
221+
styleObjectForEach({ color: 'red;background:blue' }, fn)
222+
expect(fn).not.toBeCalled()
223+
})
224+
225+
it('keeps safe siblings while skipping unsafe ones', () => {
226+
const fn = vi.fn()
227+
styleObjectForEach({ color: 'red;background:blue', backgroundColor: 'white' }, fn)
228+
expect(fn).toBeCalledTimes(1)
229+
expect(fn).toBeCalledWith('background-color', 'white')
230+
})
231+
232+
it('does not throw for unsafe values', () => {
233+
expect(() => styleObjectForEach({ color: 'a;b' }, () => {})).not.toThrow()
234+
})
235+
236+
it('keeps semicolons inside quoted strings', () => {
237+
const fn = vi.fn()
238+
styleObjectForEach({ fontFamily: '"a;b", sans-serif' }, fn)
239+
expect(fn).toBeCalledWith('font-family', '"a;b", sans-serif')
240+
})
241+
242+
test.each([
243+
['LF', '\n'],
244+
['CR', '\r'],
245+
['FF', '\f'],
246+
])('skips quoted strings that contain %s before a declaration separator', (_, newline) => {
247+
const fn = vi.fn()
248+
styleObjectForEach(
249+
{ fontFamily: `"${newline};background:url(https://example.com/a.png)` },
250+
fn
251+
)
252+
expect(fn).not.toBeCalled()
253+
})
254+
255+
it('keeps semicolons inside CSS functions', () => {
256+
const fn = vi.fn()
257+
styleObjectForEach({ backgroundImage: 'url("data:image/svg+xml;utf8,<svg></svg>")' }, fn)
258+
expect(fn).toBeCalledWith('background-image', 'url("data:image/svg+xml;utf8,<svg></svg>")')
259+
})
260+
261+
test.each([['square brackets', 'red[;background:blue]']])(
262+
'keeps semicolons inside CSS simple blocks with %s',
263+
(_, value) => {
264+
const fn = vi.fn()
265+
styleObjectForEach({ color: value }, fn)
266+
expect(fn).toBeCalledWith('color', value)
267+
}
268+
)
269+
270+
test.each([
271+
['opening curly block', 'red{;background:blue}'],
272+
['closing curly block', 'red};background:blue'],
273+
])('skips CSS values containing %s', (_, value) => {
274+
const fn = vi.fn()
275+
styleObjectForEach({ color: value, backgroundColor: 'white' }, fn)
276+
expect(fn).toBeCalledTimes(1)
277+
expect(fn).toBeCalledWith('background-color', 'white')
278+
})
279+
280+
test.each([
281+
['square brackets', 'red[;background:blue];position:fixed'],
282+
['curly braces', 'red{;background:blue};position:fixed'],
283+
])('skips top-level semicolons after CSS simple blocks with %s', (_, value) => {
284+
const fn = vi.fn()
285+
styleObjectForEach({ color: value }, fn)
286+
expect(fn).not.toBeCalled()
287+
})
288+
289+
it('skips top-level semicolons after CSS comments', () => {
290+
const fn = vi.fn()
291+
styleObjectForEach({ color: 'red/*(*/;background:blue;position:fixed;top:0' }, fn)
292+
expect(fn).not.toBeCalled()
293+
})
294+
295+
test.each([
296+
['comment', 'red/*'],
297+
['double-quoted string', '"abc'],
298+
['single-quoted string', "'abc"],
299+
['function block', 'red('],
300+
['square block', 'red['],
301+
['curly block', 'red{'],
302+
['unmatched function closer', 'red)'],
303+
['unmatched square closer', 'red]'],
304+
['unmatched curly closer', 'red}'],
305+
['mismatched simple block closer', 'red[)'],
306+
['escape', 'red\\'],
307+
])('skips unterminated CSS %s that can swallow following declarations', (_, value) => {
308+
const fn = vi.fn()
309+
styleObjectForEach({ color: value, display: 'none' }, fn)
310+
expect(fn).toBeCalledTimes(1)
311+
expect(fn).toBeCalledWith('display', 'none')
312+
})
313+
314+
test.each([
315+
['comment that closes exactly at end of value', 'red/* hi */'],
316+
['CSS hex escape that decodes to a delimiter byte', 'red\\3b '],
317+
['vertical tab inside a quoted string', '"\vfoo"'],
318+
['trailing solidus that is not a comment start', 'a/'],
319+
])('keeps CSS-safe edge case — %s', (_, value) => {
320+
const fn = vi.fn()
321+
styleObjectForEach({ color: value }, fn)
322+
expect(fn).toBeCalledWith('color', value)
323+
})
324+
325+
it('skips style property names that can break declaration boundaries', () => {
326+
const fn = vi.fn()
327+
styleObjectForEach({ 'color;background-image': 'url(https://example.com/a.png)' }, fn)
328+
styleObjectForEach({ 'color:background': 'red' }, fn)
329+
expect(fn).not.toBeCalled()
330+
})
331+
332+
it('keeps safe property names that use digits', () => {
333+
const fn = vi.fn()
334+
styleObjectForEach({ '--my-var-1': '15px', '--myVar-2': '20px' }, fn)
335+
expect(fn).toHaveBeenNthCalledWith(1, '--my-var-1', '15px')
336+
expect(fn).toHaveBeenNthCalledWith(2, '--myVar-2', '20px')
337+
})
338+
339+
it('keeps vendor-prefixed property names', () => {
340+
const fn = vi.fn()
341+
styleObjectForEach({ WebkitLineClamp: '2' }, fn)
342+
expect(fn).toBeCalledWith('-webkit-line-clamp', '2')
343+
})
344+
345+
it('keeps validating style property names after the valid style property name cache is reset', () => {
346+
for (let i = 0; i < 1025; i++) {
347+
styleObjectForEach({ [`--cache-${i}`]: '1px' }, () => {})
348+
}
349+
350+
const fn = vi.fn()
351+
styleObjectForEach({ '--after-reset-1': '2px', 'color:background': 'red' }, fn)
352+
expect(fn).toBeCalledTimes(1)
353+
expect(fn).toBeCalledWith('--after-reset-1', '2px')
354+
})
355+
356+
it('skips unsupported runtime values without throwing', () => {
357+
const fn = vi.fn()
358+
expect(() => styleObjectForEach({ color: false, backgroundColor: 'white' }, fn)).not.toThrow()
359+
expect(fn).toBeCalledTimes(1)
360+
expect(fn).toBeCalledWith('background-color', 'white')
361+
})
362+
})
217363
})

0 commit comments

Comments
 (0)