Skip to content

Commit 2b8ed40

Browse files
usualomausualoma
authored
Merge commit from fork
* fix(jsx): close server-side escaping bypasses * fix(jsx): escape array roots in DOM server * fix(jsx): escape untrusted boundary objects * fix(jsx): escape untrusted fallback and provider objects
1 parent cac0c4d commit 2b8ed40

12 files changed

Lines changed: 537 additions & 54 deletions

‎src/jsx/base.test.tsx‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/** @jsxImportSource ./ */
22

3+
import type { HtmlEscapedString } from '../utils/html'
34
import type { Child, JSXNode } from './base'
45
// eslint-disable-next-line @typescript-eslint/no-unused-vars
56
import { cloneElement, jsx, Fragment } from './base'
@@ -49,4 +50,24 @@ describe('createElement', () => {
4950
const element = jsx('div', null, child) as unknown as JSXNode
5051
expect(element.toString()).toBe('<div><span>inner</span></div>')
5152
})
53+
54+
it('should escape a string returned by an asynchronous component', async () => {
55+
const Async = async () => '<img src=x onerror=alert(1)>' as unknown as HtmlEscapedString
56+
57+
expect(
58+
String(
59+
await (
60+
<div>
61+
<Async />
62+
</div>
63+
).toString()
64+
)
65+
).toBe('<div>&lt;img src=x onerror=alert(1)&gt;</div>')
66+
})
67+
68+
it('should omit an unsupported object child', () => {
69+
const object = { toString: () => 'plain-text' }
70+
71+
expect((<div>{object as never}</div>).toString()).toBe('<div></div>')
72+
})
5273
})

‎src/jsx/base.ts‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,14 @@ const resolveFunctionComponentResult = (
118118
suspendedContext?: SuspendedContext
119119
): Promise<string> =>
120120
result.then((resolved) => {
121-
if (!Array.isArray(resolved) && !(resolved instanceof JSXNode)) {
121+
if (
122+
typeof resolved !== 'string' &&
123+
!Array.isArray(resolved) &&
124+
!(resolved instanceof JSXNode)
125+
) {
122126
return resolved
123127
}
124-
const children = Array.isArray(resolved) ? resolved : [resolved]
128+
const children = Array.isArray(resolved) ? resolved : [resolved as Child]
125129
const render = () => {
126130
const buffer: StringBufferWithCallbacks = [''] as StringBufferWithCallbacks
127131
childrenToStringToBuffer(children, buffer)
@@ -168,6 +172,33 @@ export type Child =
168172
| undefined
169173
| boolean
170174
| Child[]
175+
176+
export const renderChildren = (children: Child[]): HtmlEscapedString | Promise<HtmlEscapedString> =>
177+
runWithRenderContext(() => {
178+
const buffer: StringBufferWithCallbacks = [''] as StringBufferWithCallbacks
179+
childrenToStringToBuffer(children, buffer)
180+
return buffer.length === 1
181+
? raw(buffer[0], buffer.callbacks)
182+
: stringBufferToString(buffer, buffer.callbacks)
183+
})
184+
185+
export const isUntrustedObject = (value: unknown): boolean =>
186+
typeof value === 'object' &&
187+
value !== null &&
188+
!Array.isArray(value) &&
189+
!(value instanceof JSXNode) &&
190+
!(value instanceof Promise) &&
191+
!(value as HtmlEscaped).isEscaped &&
192+
typeof (value as { toString?: unknown }).toString === 'function'
193+
194+
export const renderUntrustedObject = (
195+
value: unknown
196+
): HtmlEscapedString | Promise<HtmlEscapedString> => {
197+
const stringified = (value as { toString(): unknown }).toString()
198+
const escape = (result: unknown) => renderChildren([String(result)])
199+
return stringified instanceof Promise ? stringified.then(escape) : escape(stringified)
200+
}
201+
171202
export class JSXNode implements HtmlEscaped {
172203
tag: string | Function
173204
props: Props

‎src/jsx/components.test.tsx‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ import { HtmlEscapedCallbackPhase, resolveCallback as rawResolveCallback } from
77
import { ErrorBoundary } from './components'
88
import { Suspense, renderToReadableStream, StreamingContext } from './streaming'
99

10+
const unsafeHtml = '<img src=x onerror=alert(1)>'
11+
const escapedUnsafeHtml = '&lt;img src=x onerror=alert(1)&gt;'
12+
1013
function resolveCallback(template: string | HtmlEscapedString) {
1114
return rawResolveCallback(template, HtmlEscapedCallbackPhase.Stream, false, {})
1215
}
@@ -17,6 +20,21 @@ function replacementResult(html: string) {
1720
return document.body.innerHTML
1821
}
1922

23+
async function stringify(node: { toString(): string | Promise<string> }): Promise<string> {
24+
return String(
25+
await rawResolveCallback(await node.toString(), HtmlEscapedCallbackPhase.Stringify, false, {})
26+
)
27+
}
28+
29+
async function drainStream(stream: ReadableStream<Uint8Array>): Promise<string> {
30+
const decoder = new TextDecoder()
31+
let result = ''
32+
for await (const chunk of stream) {
33+
result += decoder.decode(chunk)
34+
}
35+
return result
36+
}
37+
2038
const Fallback = () => <div>Out Of Service</div>
2139

2240
describe('ErrorBoundary', () => {
@@ -798,4 +816,121 @@ d.remove()
798816
suspenseCounter--
799817
})
800818
})
819+
820+
describe('escaping', () => {
821+
const Broken = () => {
822+
throw new Error('boom')
823+
}
824+
825+
it('escapes strings when an asynchronous sibling is present', async () => {
826+
const Async = async () => <span>done</span>
827+
const node = (
828+
<ErrorBoundary fallback='error'>
829+
{unsafeHtml}
830+
<Async />
831+
</ErrorBoundary>
832+
)
833+
834+
expect(await stringify(node)).toBe(`${escapedUnsafeHtml}<span>done</span>`)
835+
const streamed = await drainStream(renderToReadableStream(node))
836+
expect(streamed).toContain(escapedUnsafeHtml)
837+
expect(streamed).not.toContain(unsafeHtml)
838+
})
839+
840+
it('escapes untrusted object fallbacks', async () => {
841+
const object = { toString: () => unsafeHtml }
842+
const asyncObject = { toString: async () => unsafeHtml }
843+
const rawObject = { toString: () => raw(unsafeHtml) }
844+
845+
for (const fallback of [object, asyncObject, rawObject]) {
846+
expect(
847+
await stringify(
848+
<ErrorBoundary fallback={fallback as never}>
849+
<Broken />
850+
</ErrorBoundary>
851+
)
852+
).toBe(escapedUnsafeHtml)
853+
}
854+
})
855+
856+
it('preserves an explicitly trusted fallback', async () => {
857+
expect(
858+
await stringify(
859+
<ErrorBoundary fallback={raw('<strong>trusted</strong>')}>
860+
<Broken />
861+
</ErrorBoundary>
862+
)
863+
).toBe('<strong>trusted</strong>')
864+
})
865+
866+
it('does not interpret replacement patterns in content', async () => {
867+
const content = "literal $& $` $'"
868+
const Async = async () => <span>done</span>
869+
870+
expect(
871+
await stringify(
872+
<ErrorBoundary fallback='error'>
873+
{content}
874+
<Async />
875+
</ErrorBoundary>
876+
)
877+
).toBe('literal $&amp; $` $&#39;<span>done</span>')
878+
})
879+
880+
it('escapes untrusted object children', async () => {
881+
const object = { toString: () => unsafeHtml }
882+
const asyncObject = { toString: async () => unsafeHtml }
883+
const rawObject = { toString: () => raw(unsafeHtml) }
884+
885+
for (const child of [object, asyncObject, rawObject]) {
886+
const node = <ErrorBoundary fallback='error'>{child as never}</ErrorBoundary>
887+
expect(await stringify(node)).toBe(escapedUnsafeHtml)
888+
const streamed = await drainStream(renderToReadableStream(node))
889+
expect(streamed).toContain(escapedUnsafeHtml)
890+
expect(streamed).not.toContain(unsafeHtml)
891+
}
892+
})
893+
894+
it('preserves explicitly trusted children', async () => {
895+
expect(
896+
await stringify(
897+
<ErrorBoundary fallback='error'>{raw('<strong>trusted</strong>')}</ErrorBoundary>
898+
)
899+
).toBe('<strong>trusted</strong>')
900+
})
901+
902+
it('keeps benign object stringification visible', async () => {
903+
expect(await stringify(<ErrorBoundary fallback='error'>{{} as never}</ErrorBoundary>)).toBe(
904+
'[object Object]'
905+
)
906+
})
907+
908+
it('escapes nested fallback arrays', async () => {
909+
expect(
910+
await stringify(
911+
<ErrorBoundary fallback={[[unsafeHtml]]}>
912+
<Broken />
913+
</ErrorBoundary>
914+
)
915+
).toBe(escapedUnsafeHtml)
916+
})
917+
918+
it('keeps the existing falsy fallbackRender behavior', async () => {
919+
expect(
920+
await stringify(
921+
<ErrorBoundary fallbackRender={() => 0}>
922+
<Broken />
923+
</ErrorBoundary>
924+
)
925+
).toBe('')
926+
})
927+
928+
it('keeps a direct Promise child unchanged', async () => {
929+
expect(
930+
await stringify(
931+
<ErrorBoundary fallback='error'>{Promise.resolve('resolved')}</ErrorBoundary>
932+
)
933+
).toBe('[object Promise]')
934+
})
935+
})
801936
})

‎src/jsx/components.ts‎

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { raw } from '../helper/html'
22
import type { HtmlEscapedCallback, HtmlEscapedString } from '../utils/html'
33
import { HtmlEscapedCallbackPhase, resolveCallback } from '../utils/html'
4-
import { jsx, Fragment } from './base'
4+
import { jsx, Fragment, isUntrustedObject, renderChildren, renderUntrustedObject } from './base'
55
import { DOM_RENDERER } from './constants'
66
import { captureRenderContext, useContext } from './context'
77
import { ErrorBoundary as ErrorBoundaryDomRenderer } from './dom/components'
@@ -13,9 +13,7 @@ let errorBoundaryCounter = 0
1313

1414
export const childrenToString = async (children: Child[]): Promise<HtmlEscapedString[]> => {
1515
try {
16-
return children
17-
.flat()
18-
.map((c) => (c == null || typeof c === 'boolean' ? '' : c.toString())) as HtmlEscapedString[]
16+
return children.flat().map(resolveChildEarly) as HtmlEscapedString[]
1917
} catch (e) {
2018
if (e instanceof Promise) {
2119
// Capture before `await`: on the fallback path the render context is
@@ -29,18 +27,16 @@ export const childrenToString = async (children: Child[]): Promise<HtmlEscapedSt
2927
}
3028
}
3129

32-
const resolveChildEarly = (c: Child): HtmlEscapedString | Promise<HtmlEscapedString> => {
33-
if (c == null || typeof c === 'boolean') {
30+
const resolveChildEarly = (child: Child): HtmlEscapedString | Promise<HtmlEscapedString> => {
31+
if (child == null || typeof child === 'boolean') {
3432
return '' as HtmlEscapedString
35-
} else if (typeof c === 'string') {
36-
return c as HtmlEscapedString
33+
} else if (typeof child === 'string' || Array.isArray(child)) {
34+
return renderChildren([child])
35+
} else if (isUntrustedObject(child)) {
36+
return renderUntrustedObject(child)
3737
} else {
38-
const str = c.toString()
39-
if (!(str instanceof Promise)) {
40-
return raw(str)
41-
} else {
42-
return str as Promise<HtmlEscapedString>
43-
}
38+
const str = child.toString()
39+
return str instanceof Promise ? (str as Promise<HtmlEscapedString>) : raw(str)
4440
}
4541
}
4642

@@ -76,20 +72,21 @@ export const ErrorBoundary: FC<
7672
const resolveFallbackStr = (): Promise<HtmlEscapedString | string | undefined> =>
7773
(fallbackStrPromise ||= (async () => {
7874
const awaitedFallback = await fallback
79-
if (typeof awaitedFallback === 'string') {
80-
return awaitedFallback
81-
} else {
82-
const fallbackResult = await getResume()(() => awaitedFallback?.toString())
83-
if (typeof fallbackResult === 'string') {
84-
// Don't apply `raw` to undefined/null/boolean. Preserve callbacks from
85-
// the stringified result, or the original thenable for plain strings.
86-
return raw(
87-
fallbackResult,
88-
(fallbackResult as HtmlEscapedString).callbacks ||
89-
(awaitedFallback as unknown as HtmlEscapedString)?.callbacks
90-
)
91-
}
75+
if (awaitedFallback === null || awaitedFallback === undefined) {
76+
return
77+
}
78+
if (typeof awaitedFallback === 'string' || Array.isArray(awaitedFallback)) {
79+
return getResume()(() => renderChildren([awaitedFallback]))
9280
}
81+
if (isUntrustedObject(awaitedFallback)) {
82+
return getResume()(() => renderUntrustedObject(awaitedFallback))
83+
}
84+
const fallbackResult = await getResume()(() => awaitedFallback.toString())
85+
return raw(
86+
fallbackResult,
87+
(fallbackResult as HtmlEscapedString).callbacks ||
88+
(awaitedFallback as unknown as HtmlEscapedString).callbacks
89+
)
9390
})())
9491
const renderFallback = async (error: Error): Promise<HtmlEscapedString> => {
9592
const fallbackStr = await resolveFallbackStr()
@@ -100,9 +97,7 @@ export const ErrorBoundary: FC<
10097
? fallbackStr
10198
: (fallbackRender && jsx(Fragment, {}, fallbackRender(error) as HtmlEscapedString)) || ''
10299
) as HtmlEscapedString
103-
const fallbackResString = await Fragment({
104-
children: fallbackRes,
105-
}).toString()
100+
const fallbackResString = await Fragment({ children: fallbackRes }).toString()
106101
return raw(
107102
fallbackResString,
108103
(fallbackResString as HtmlEscapedString).callbacks || fallbackRes.callbacks
@@ -142,7 +137,7 @@ export const ErrorBoundary: FC<
142137
const fallbackResString = await renderFallback(error)
143138
const fallbackCallbacks = fallbackResString.callbacks
144139
if (buffer) {
145-
buffer[0] = buffer[0].replace(replaceRe, fallbackResString)
140+
buffer[0] = buffer[0].replace(replaceRe, () => fallbackResString)
146141
return fallbackCallbacks?.length ? raw('', fallbackCallbacks) : ''
147142
}
148143
return raw(
@@ -188,7 +183,7 @@ d.parentElement.insertBefore(c.content,d.nextSibling)
188183

189184
if (htmlArray.every((html) => !(html as HtmlEscapedString).callbacks?.length)) {
190185
if (buffer) {
191-
buffer[0] = buffer[0].replace(replaceRe, content)
186+
buffer[0] = buffer[0].replace(replaceRe, () => content)
192187
}
193188
return html
194189
}

‎src/jsx/context.ts‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { raw } from '../helper/html'
22
import type { HtmlEscapedString } from '../utils/html'
3-
import { JSXFragmentNode } from './base'
3+
import { isUntrustedObject, JSXFragmentNode, renderChildren, renderUntrustedObject } from './base'
44
import { DOM_RENDERER } from './constants'
55
import { createContextProviderFunction } from './dom/context'
66
import type { FC, PropsWithChildren } from './'
@@ -217,26 +217,31 @@ export const createContext = <T>(defaultValue: T): Context<T> => {
217217
// below may run later and must operate on the same array as the push.
218218
const contextValues = getContextValuesIn(getCurrentStore(), context)
219219
contextValues.push(props.value)
220-
let string
220+
let rendered
221221
try {
222-
string = props.children
223-
? (Array.isArray(props.children)
224-
? new JSXFragmentNode('', {}, props.children)
222+
rendered =
223+
typeof props.children === 'string'
224+
? renderChildren([props.children])
225+
: isUntrustedObject(props.children)
226+
? renderUntrustedObject(props.children)
225227
: props.children
226-
).toString()
227-
: ''
228+
? (Array.isArray(props.children)
229+
? new JSXFragmentNode('', {}, props.children)
230+
: props.children
231+
).toString()
232+
: raw('')
228233
} catch (e) {
229234
contextValues.pop()
230235
throw e
231236
}
232237

233-
if (string instanceof Promise) {
234-
return string
238+
if (rendered instanceof Promise) {
239+
return rendered
235240
.finally(() => contextValues.pop())
236241
.then((resString) => raw(resString, (resString as HtmlEscapedString).callbacks))
237242
} else {
238243
contextValues.pop()
239-
return raw(string)
244+
return raw(rendered)
240245
}
241246
}) as Context<T>
242247
context.values = values

0 commit comments

Comments
 (0)