From 7e742eb3a1001542d795776c0317d47df8b9d64e Mon Sep 17 00:00:00 2001 From: Shai Reznik Date: Tue, 25 Jun 2024 00:31:30 +0300 Subject: [PATCH] Merge pull request from GHSA-2rwj-7xq8-4gx4 * chore: added more escapes * refactor: consolidated escape fn --- .../qwik/src/core/render/ssr/render-ssr.ts | 100 ++++++++---------- .../src/core/render/ssr/render-ssr.unit.tsx | 20 ++-- 2 files changed, 55 insertions(+), 65 deletions(-) diff --git a/packages/qwik/src/core/render/ssr/render-ssr.ts b/packages/qwik/src/core/render/ssr/render-ssr.ts index 9267f69b212..cd970f7757f 100644 --- a/packages/qwik/src/core/render/ssr/render-ssr.ts +++ b/packages/qwik/src/core/render/ssr/render-ssr.ts @@ -1,10 +1,42 @@ +import { + createContainerState, + getEventName, + setRef, + type ContainerState, +} from '../../container/container'; +import { assertDefined } from '../../error/assert'; +import { QError_canNotRenderHTML, qError } from '../../error/error'; +import { serializeQRLs } from '../../qrl/qrl'; +import { Q_CTX, _IMMUTABLE, _IMMUTABLE_PREFIX } from '../../state/constants'; +import { + HOST_FLAG_DIRTY, + HOST_FLAG_DYNAMIC, + HOST_FLAG_NEED_ATTACH_LISTENER, + createContext, + type QContext, +} from '../../state/context'; +import { + PREVENT_DEFAULT, + groupListeners, + isOnProp, + setEvent, + type Listener, +} from '../../state/listeners'; +import { isSignal } from '../../state/signal'; +import { createPropsState, createProxy } from '../../state/store'; +import { serializeSStyle } from '../../style/qrl-styles'; +import { invoke, newInvokeContext, trackSignal, type InvokeContext } from '../../use/use-core'; +import { EMPTY_OBJ } from '../../util/flyweight'; +import { logError, logWarn } from '../../util/log'; +import { ELEMENT_ID, OnRenderProp, QScopedStyle, QSlot, QSlotS, QStyle } from '../../util/markers'; import { isPromise, maybeThen } from '../../util/promises'; -import { type InvokeContext, newInvokeContext, invoke, trackSignal } from '../../use/use-core'; -import { Virtual, _jsxC, _jsxQ, createJSXError, isJSXNode } from '../jsx/jsx-runtime'; +import { qDev, qInspector, seal } from '../../util/qdev'; import { isArray, isFunction, isString, type ValueOrPromise } from '../../util/types'; -import type { FunctionComponent, JSXNode, JSXOutput } from '../jsx/types/jsx-node'; +import { version } from '../../version'; +import type { QwikElement } from '../dom/virtual-element'; import { createRenderContext, + dangerouslySetInnerHTML, executeComponent, getNextIndex, isAriaAttribute, @@ -14,44 +46,12 @@ import { shouldWrapFunctional, static_subtree, stringifyStyle, - dangerouslySetInnerHTML, } from '../execute-component'; -import { ELEMENT_ID, OnRenderProp, QScopedStyle, QSlot, QSlotS, QStyle } from '../../util/markers'; +import { Virtual, _jsxC, _jsxQ, createJSXError, isJSXNode } from '../jsx/jsx-runtime'; +import type { FunctionComponent, JSXNode, JSXOutput } from '../jsx/types/jsx-node'; +import type { ClassList, JSXChildren } from '../jsx/types/jsx-qwik-attributes'; import { InternalSSRStream, SSRRaw } from '../jsx/utils.public'; -import { logError, logWarn } from '../../util/log'; -import { - groupListeners, - isOnProp, - type Listener, - PREVENT_DEFAULT, - setEvent, -} from '../../state/listeners'; -import { version } from '../../version'; -import { - type ContainerState, - createContainerState, - setRef, - getEventName, -} from '../../container/container'; import type { RenderContext } from '../types'; -import { assertDefined } from '../../error/assert'; -import { serializeSStyle } from '../../style/qrl-styles'; -import { qDev, qInspector, seal } from '../../util/qdev'; -import { qError, QError_canNotRenderHTML } from '../../error/error'; -import { isSignal } from '../../state/signal'; -import { serializeQRLs } from '../../qrl/qrl'; -import type { QwikElement } from '../dom/virtual-element'; -import { EMPTY_OBJ } from '../../util/flyweight'; -import { - createContext, - HOST_FLAG_DIRTY, - HOST_FLAG_NEED_ATTACH_LISTENER, - HOST_FLAG_DYNAMIC, - type QContext, -} from '../../state/context'; -import { createPropsState, createProxy } from '../../state/store'; -import { Q_CTX, _IMMUTABLE, _IMMUTABLE_PREFIX } from '../../state/constants'; -import type { ClassList, JSXChildren } from '../jsx/types/jsx-qwik-attributes'; const FLUSH_COMMENT = ''; @@ -603,7 +603,7 @@ const renderNode = ( } } else { openingElement += - ' ' + (value === true ? prop : prop + '="' + escapeAttr(attrValue) + '"'); + ' ' + (value === true ? prop : prop + '="' + escapeHtml(attrValue) + '"'); } } }; @@ -732,7 +732,7 @@ This goes against the HTML spec: https://html.spec.whatwg.org/multipage/dom.html } if (classStr) { - openingElement += ' class="' + escapeAttr(classStr) + '"'; + openingElement += ' class="' + escapeHtml(classStr) + '"'; } if (listeners.length > 0) { @@ -750,7 +750,7 @@ This goes against the HTML spec: https://html.spec.whatwg.org/multipage/dom.html } } if (key != null) { - openingElement += ' q:key="' + escapeAttr(key) + '"'; + openingElement += ' q:key="' + escapeHtml(key) + '"'; } if (hasRef || useSignal || listeners.length > 0) { if (hasRef || useSignal || listenersNeedId(listeners)) { @@ -766,7 +766,7 @@ This goes against the HTML spec: https://html.spec.whatwg.org/multipage/dom.html if (qDev && qInspector && node.dev && !(flags & IS_HEAD)) { const sanitizedFileName = node?.dev?.fileName?.replace(/\\/g, '/'); if (sanitizedFileName && !/data-qwik-inspector/.test(openingElement)) { - openingElement += ` data-qwik-inspector="${escapeAttr( + openingElement += ` data-qwik-inspector="${escapeHtml( `${sanitizedFileName}:${node.dev.lineNumber}:${node.dev.columnNumber}` )}"`; } @@ -1180,8 +1180,7 @@ export interface ServerDocument { createElement(tagName: string): any; } -const ESCAPE_HTML = /[&<>]/g; -const ESCAPE_ATTRIBUTES = /[&"]/g; +const ESCAPE_HTML = /[&<>'"]/g; export const registerQwikEvent = (prop: string, containerState: ContainerState) => { containerState.$events$.add(getEventName(prop)); @@ -1196,19 +1195,10 @@ const escapeHtml = (s: string) => { return '<'; case '>': return '>'; - default: - return ''; - } - }); -}; - -const escapeAttr = (s: string) => { - return s.replace(ESCAPE_ATTRIBUTES, (c) => { - switch (c) { - case '&': - return '&'; case '"': return '"'; + case "'": + return '''; default: return ''; } diff --git a/packages/qwik/src/core/render/ssr/render-ssr.unit.tsx b/packages/qwik/src/core/render/ssr/render-ssr.unit.tsx index c1870e481ea..ba2345b176c 100644 --- a/packages/qwik/src/core/render/ssr/render-ssr.unit.tsx +++ b/packages/qwik/src/core/render/ssr/render-ssr.unit.tsx @@ -1,5 +1,6 @@ import { format } from 'prettier'; +import { expect, test, vi } from 'vitest'; import type { StreamWriter } from '../../../server/types'; import { component$ } from '../../component/component.public'; import { inlinedQrl } from '../../qrl/qrl'; @@ -7,17 +8,16 @@ import { $ } from '../../qrl/qrl.public'; import { createContextId, useContext, useContextProvider } from '../../use/use-context'; import { useOn, useOnDocument, useOnWindow } from '../../use/use-on'; import { Resource, useResource$ } from '../../use/use-resource'; -import { useStylesScopedQrl, useStylesQrl } from '../../use/use-styles'; -import { useVisibleTask$, useTask$ } from '../../use/use-task'; +import { useSignal } from '../../use/use-signal'; +import { useStore } from '../../use/use-store.public'; +import { useStylesQrl, useStylesScopedQrl } from '../../use/use-styles'; +import { useTask$, useVisibleTask$ } from '../../use/use-task'; import { delay } from '../../util/promises'; -import { SSRComment, SSRRaw } from '../jsx/utils.public'; -import { Slot } from '../jsx/slot.public'; import { HTMLFragment, jsx } from '../jsx/jsx-runtime'; -import { _renderSSR, type RenderSSROptions } from './render-ssr'; -import { useStore } from '../../use/use-store.public'; -import { useSignal } from '../../use/use-signal'; -import { expect, test, vi } from 'vitest'; +import { Slot } from '../jsx/slot.public'; import type { JSXOutput } from '../jsx/types/jsx-node'; +import { SSRComment, SSRRaw } from '../jsx/utils.public'; +import { _renderSSR, type RenderSSROptions } from './render-ssr'; test('render attributes', async () => { await testSSR( @@ -769,7 +769,7 @@ test('using component props', async () => {
-
MyCmp{"id":"12","host:prop":"attribute","innerHTML":"123","dangerouslySetInnerHTML":"432","onClick":"lazy.js","prop":"12"}
+
MyCmp{"id":"12","host:prop":"attribute","innerHTML":"123","dangerouslySetInnerHTML":"432","onClick":"lazy.js","prop":"12"}
@@ -1509,7 +1509,7 @@ test('cleanse class attribute', async () => { }; await testSSR( , - '' + '' ); });