From 8da69e0f77ea820a6845a4c3c025b6af3861d523 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Sat, 4 Apr 2026 08:05:27 +0200 Subject: [PATCH] Fix stored XSS via unsafe URL protocols in href attributes (#19282) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Fixes **GHSA-7w89-7q26-gj7q**: stored XSS via `javascript:` URIs in BlockNote `FileBlock` `props.url`, rendered as a clickable ``. - Audited the full codebase and hardened **all** surfaces where user-controlled URLs are rendered as `href` or passed to `window.open`. - Applies defense-in-depth: server-side input validation + client-side render-time checks + lint rules to prevent regressions. ### Changes **New utility** — `isSafeUrl` (`~/utils/isSafeUrl.ts`): Allowlists `http:`, `https:`, `mailto:`, `tel:` protocols and relative paths (`/`). Returns `false` for `javascript:`, `data:`, `vbscript:`, etc. **Server-side** — `validateBlocknoteFieldOrThrow`: - Recursively walks all blocks and validates `props.url` and inline link `href` values - Rejects payloads with unsafe URL protocols at save time (before data is stored) **Client-side** — 8 components hardened: | Component | Fix | |-----------|-----| | `FileBlock` (reported vuln) | `isSafeUrl` gate, fixed `target="__blank"` → `_blank`, added `rel="noopener noreferrer"` | | `LazyMarkdownRenderer` | `isSafeUrl` gate on markdown ``, added `target`/`rel` | | `EditLinkPopover` (TipTap) | Validates + auto-prefixes `https://`, rejects unsafe URLs | | `LinkBubbleMenu` (TipTap) | `isSafeUrl` gate on `window.open`, added `noopener,noreferrer` | | `AttachmentRow` | `isSafeUrl` gate on file attachment `href` | | `URLDisplay` / `LinkDisplay` | `isSafeUrl` as second check after `startsWith('http')` | | `IframeWidget` | `isSafeUrl` gate on `src`, shows error state for unsafe URLs | | `InformationBannerMaintenance` | `isSafeUrl` gate on `window.open` | **Lint rules** — `.oxlintrc.json`: - `no-script-url: error` — catches `javascript:` string literals - `react/jsx-no-script-url: error` — catches `javascript:` in JSX href attributes ## Test plan - [ ] Create a note via GraphQL mutation with `"url": "javascript:void(alert(1))"` in a file block — should be rejected by server validation - [ ] Verify existing file attachments in notes still render and are clickable - [ ] Verify TipTap link insertion works for normal `https://` URLs - [ ] Verify TipTap link insertion rejects `javascript:` URIs - [ ] Verify markdown links in AI chat render correctly for safe URLs - [ ] Verify URL/Link field displays still work for normal URLs - [ ] Verify iframe widget rejects non-http(s) URLs Made with [Cursor](https://cursor.com) --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 --- packages/twenty-front/.oxlintrc.json | 2 ++ .../files/components/AttachmentRow.tsx | 4 +-- .../components/EditLinkPopover.tsx | 17 ++++++----- .../components/LinkBubbleMenu.tsx | 7 ++++- .../ai/components/LazyMarkdownRenderer.tsx | 10 +++---- .../blocknote-editor/blocks/FileBlock.tsx | 12 ++++++-- .../InformationBannerMaintenance.tsx | 7 ++--- .../iframe/components/IframeWidget.tsx | 9 ++++-- .../field/display/components/LinkDisplay.tsx | 7 ++--- .../field/display/components/URLDisplay.tsx | 7 ++--- .../validate-rich-text-field-or-throw.util.ts | 29 +++++++++++++++++++ packages/twenty-shared/src/utils/index.ts | 2 ++ .../twenty-shared/src/utils/url/getSafeUrl.ts | 17 +++++++++++ packages/twenty-shared/src/utils/url/index.ts | 2 ++ .../twenty-shared/src/utils/url/isSafeUrl.ts | 15 ++++++++++ 15 files changed, 110 insertions(+), 37 deletions(-) create mode 100644 packages/twenty-shared/src/utils/url/getSafeUrl.ts create mode 100644 packages/twenty-shared/src/utils/url/isSafeUrl.ts diff --git a/packages/twenty-front/.oxlintrc.json b/packages/twenty-front/.oxlintrc.json index 5b84f3ee97..e4658bd4e7 100644 --- a/packages/twenty-front/.oxlintrc.json +++ b/packages/twenty-front/.oxlintrc.json @@ -21,6 +21,7 @@ "rules": { "func-style": ["error", "declaration", { "allowArrowFunctions": true }], "no-console": ["warn", { "allow": ["group", "groupCollapsed", "groupEnd"] }], + "no-script-url": "error", "no-control-regex": "off", "no-debugger": "error", "no-duplicate-imports": "error", @@ -58,6 +59,7 @@ "react/jsx-uses-react": "off", "react/react-in-jsx-scope": "off", "react/jsx-no-useless-fragment": "off", + "react/jsx-no-script-url": "error", "react/jsx-props-no-spreading": ["error", { "explicitSpread": "ignore" }], "react-hooks/rules-of-hooks": "error", diff --git a/packages/twenty-front/src/modules/activities/files/components/AttachmentRow.tsx b/packages/twenty-front/src/modules/activities/files/components/AttachmentRow.tsx index 0d938c5725..369b8cd5b6 100644 --- a/packages/twenty-front/src/modules/activities/files/components/AttachmentRow.tsx +++ b/packages/twenty-front/src/modules/activities/files/components/AttachmentRow.tsx @@ -11,7 +11,7 @@ import { getFileCategoryFromExtension } from '@/object-record/record-field/ui/ut import { SettingsTextInput } from '@/ui/input/components/SettingsTextInput'; import { styled } from '@linaria/react'; import { useState, useContext } from 'react'; -import { isDefined } from 'twenty-shared/utils'; +import { getSafeUrl, isDefined } from 'twenty-shared/utils'; import { type AttachmentWithFile } from '@/activities/files/utils/filterAttachmentsWithFile'; import { FileIcon } from '@/file/components/FileIcon'; @@ -186,7 +186,7 @@ export const AttachmentRow = ({ diff --git a/packages/twenty-front/src/modules/advanced-text-editor/components/EditLinkPopover.tsx b/packages/twenty-front/src/modules/advanced-text-editor/components/EditLinkPopover.tsx index c1a61fff77..c1c1b71b99 100644 --- a/packages/twenty-front/src/modules/advanced-text-editor/components/EditLinkPopover.tsx +++ b/packages/twenty-front/src/modules/advanced-text-editor/components/EditLinkPopover.tsx @@ -8,7 +8,7 @@ import { useLingui } from '@lingui/react/macro'; import { isNonEmptyString } from '@sniptt/guards'; import { type Editor } from '@tiptap/core'; import { useId, useState, type FocusEvent, type FormEvent } from 'react'; -import { isDefined } from 'twenty-shared/utils'; +import { getSafeUrl, isDefined } from 'twenty-shared/utils'; import { IconLink, IconPencil } from 'twenty-ui/display'; type EditLinkPopoverProps = { @@ -33,15 +33,16 @@ export const EditLinkPopover = ({ ) => { event.preventDefault(); - if (!isDefined(value)) { + if (!isDefined(value) || !isNonEmptyString(value)) { editor.chain().focus().extendMarkRange('link').unsetLink().run(); } else { - editor - .chain() - .focus() - .extendMarkRange('link') - .setLink({ href: value }) - .run(); + const href = getSafeUrl(value); + + if (!href) { + return; + } + + editor.chain().focus().extendMarkRange('link').setLink({ href }).run(); } toggleDropdown({ dropdownComponentInstanceIdFromProps: dropdownId }); diff --git a/packages/twenty-front/src/modules/advanced-text-editor/components/LinkBubbleMenu.tsx b/packages/twenty-front/src/modules/advanced-text-editor/components/LinkBubbleMenu.tsx index 01cd753e15..9a9f531128 100644 --- a/packages/twenty-front/src/modules/advanced-text-editor/components/LinkBubbleMenu.tsx +++ b/packages/twenty-front/src/modules/advanced-text-editor/components/LinkBubbleMenu.tsx @@ -5,6 +5,7 @@ import { type Editor } from '@tiptap/core'; import { useEditorState } from '@tiptap/react'; import { BubbleMenu } from '@tiptap/react/menus'; import { IconExternalLink, IconLinkOff } from 'twenty-ui/display'; +import { getSafeUrl } from 'twenty-shared/utils'; type LinkBubbleMenuProps = { editor: Editor; @@ -28,7 +29,11 @@ export const LinkBubbleMenu = ({ editor }: LinkBubbleMenuProps) => { { Icon: IconExternalLink, onClick: () => { - window.open(state.linkHref, '_blank'); + const safeHref = getSafeUrl(state.linkHref); + + if (safeHref) { + window.open(safeHref, '_blank', 'noopener,noreferrer'); + } }, }, { diff --git a/packages/twenty-front/src/modules/ai/components/LazyMarkdownRenderer.tsx b/packages/twenty-front/src/modules/ai/components/LazyMarkdownRenderer.tsx index 51cc1fb56b..315c21ad5b 100644 --- a/packages/twenty-front/src/modules/ai/components/LazyMarkdownRenderer.tsx +++ b/packages/twenty-front/src/modules/ai/components/LazyMarkdownRenderer.tsx @@ -18,7 +18,7 @@ import { useContext, } from 'react'; import Skeleton, { SkeletonTheme } from 'react-loading-skeleton'; -import { isDefined } from 'twenty-shared/utils'; +import { getSafeUrl, isDefined } from 'twenty-shared/utils'; import { ThemeContext } from 'twenty-ui/theme-constants'; const TextWithRecordLinks = ({ text }: { text: string }) => { @@ -121,13 +121,13 @@ const MarkdownRenderer = lazy(async () => { li: ({ children }) => (
  • {processChildrenForRecordLinks(children)}
    • ), - a: ({ children, href, title, target, rel, node: _node }) => ( + a: ({ children, href, title, node: _node }) => (     {processChildrenForRecordLinks(children)} diff --git a/packages/twenty-front/src/modules/blocknote-editor/blocks/FileBlock.tsx b/packages/twenty-front/src/modules/blocknote-editor/blocks/FileBlock.tsx index baaf1c2037..7cce9d571b 100644 --- a/packages/twenty-front/src/modules/blocknote-editor/blocks/FileBlock.tsx +++ b/packages/twenty-front/src/modules/blocknote-editor/blocks/FileBlock.tsx @@ -9,7 +9,7 @@ import { type AttachmentFileCategory } from '@/activities/files/types/Attachment import { getFileType } from '@/activities/files/utils/getFileType'; import { FileIcon } from '@/file/components/FileIcon'; import { t } from '@lingui/core/macro'; -import { isDefined } from 'twenty-shared/utils'; +import { getSafeUrl, isDefined } from 'twenty-shared/utils'; import { Button } from 'twenty-ui/input'; const StyledFileInput = styled.input` @@ -86,13 +86,19 @@ export const FileBlock = createReactBlockSpec( handleUploadAttachment?.(e.target.files[0]); }; - if (isNonEmptyString(block.props.url)) { + const safeUrl = getSafeUrl(block.props.url); + + if (safeUrl) { return ( - + {block.props.name} diff --git a/packages/twenty-front/src/modules/information-banner/components/maintenance/InformationBannerMaintenance.tsx b/packages/twenty-front/src/modules/information-banner/components/maintenance/InformationBannerMaintenance.tsx index 889eed9065..f6aa2e6e69 100644 --- a/packages/twenty-front/src/modules/information-banner/components/maintenance/InformationBannerMaintenance.tsx +++ b/packages/twenty-front/src/modules/information-banner/components/maintenance/InformationBannerMaintenance.tsx @@ -6,8 +6,7 @@ import { useMaintenanceModeBannerDismissal } from '@/information-banner/hooks/us import { useDateTimeFormat } from '@/localization/hooks/useDateTimeFormat'; import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue'; import { t } from '@lingui/core/macro'; -import { isNonEmptyString } from '@sniptt/guards'; -import { isDefined } from 'twenty-shared/utils'; +import { getSafeUrl, isDefined } from 'twenty-shared/utils'; import { IconExternalLink } from 'twenty-ui/display'; const formatMaintenanceDateTime = ( @@ -49,9 +48,7 @@ export const InformationBannerMaintenance = () => { ); const message = t`Scheduled maintenance: ${startFormatted} — ${endFormatted}`; - const maintenanceLink = isNonEmptyString(maintenanceMode.link?.trim()) - ? maintenanceMode.link.trim() - : undefined; + const maintenanceLink = getSafeUrl(maintenanceMode.link?.trim()); return ( ` @@ -79,7 +79,10 @@ export const IframeWidget = ({ widget }: IframeWidgetProps) => { setHasError(true); }; - if (hasError || !isDefined(url)) { + const safeUrl = isDefined(url) ? getSafeUrl(url) : undefined; + const isHttpUrl = isDefined(safeUrl) && /^https?:\/\//i.test(safeUrl); + + if (hasError || !isHttpUrl) { return ( @@ -98,7 +101,7 @@ export const IframeWidget = ({ widget }: IframeWidgetProps) => { )} { return <>; } - const absoluteUrl = url - ? url.startsWith('http') - ? url - : 'https://' + url - : ''; + const absoluteUrl = getSafeUrl(url) ?? ''; const displayedValue = isNonEmptyString(value.label) ? value.label diff --git a/packages/twenty-front/src/modules/ui/field/display/components/URLDisplay.tsx b/packages/twenty-front/src/modules/ui/field/display/components/URLDisplay.tsx index b266cf70e3..7a8294e5ac 100644 --- a/packages/twenty-front/src/modules/ui/field/display/components/URLDisplay.tsx +++ b/packages/twenty-front/src/modules/ui/field/display/components/URLDisplay.tsx @@ -2,6 +2,7 @@ import { type MouseEvent } from 'react'; import { LinkType, RoundedLink, SocialLink } from 'twenty-ui/navigation'; import { checkUrlType } from '~/utils/checkUrlType'; +import { getSafeUrl } from 'twenty-shared/utils'; import { EllipsisDisplay } from './EllipsisDisplay'; type URLDisplayProps = { @@ -13,11 +14,7 @@ export const URLDisplay = ({ value }: URLDisplayProps) => { event.stopPropagation(); }; - const absoluteUrl = value - ? value.startsWith('http') - ? value - : 'https://' + value - : ''; + const absoluteUrl = value ? (getSafeUrl(value) ?? '') : ''; const displayedValue = value ?? ''; diff --git a/packages/twenty-server/src/engine/api/common/common-args-processors/data-arg-processor/validator-utils/validate-rich-text-field-or-throw.util.ts b/packages/twenty-server/src/engine/api/common/common-args-processors/data-arg-processor/validator-utils/validate-rich-text-field-or-throw.util.ts index 1ae75b5ccc..39787901b8 100644 --- a/packages/twenty-server/src/engine/api/common/common-args-processors/data-arg-processor/validator-utils/validate-rich-text-field-or-throw.util.ts +++ b/packages/twenty-server/src/engine/api/common/common-args-processors/data-arg-processor/validator-utils/validate-rich-text-field-or-throw.util.ts @@ -2,6 +2,7 @@ import { inspect } from 'util'; import { msg } from '@lingui/core/macro'; import { isNonEmptyString, isNull } from '@sniptt/guards'; +import { isSafeUrl } from 'twenty-shared/utils'; import { validateRawJsonFieldOrThrow } from 'src/engine/api/common/common-args-processors/data-arg-processor/validator-utils/validate-raw-json-field-or-throw.util'; import { validateTextFieldOrThrow } from 'src/engine/api/common/common-args-processors/data-arg-processor/validator-utils/validate-text-field-or-throw.util'; @@ -10,6 +11,24 @@ import { CommonQueryRunnerExceptionCode, } from 'src/engine/api/common/common-query-runners/errors/common-query-runner.exception'; +const URL_VALUE_PATTERN = /"(?:url|href)"\s*:\s*"([^"]*)"/gi; + +const hasDangerousUrl = (json: string): boolean => { + URL_VALUE_PATTERN.lastIndex = 0; + + let match; + + while ((match = URL_VALUE_PATTERN.exec(json)) !== null) { + const url = match[1].trim(); + + if (url.length > 0 && !isSafeUrl(url)) { + return true; + } + } + + return false; +}; + const validateBlocknoteFieldOrThrow = ( value: unknown, fieldName: string, @@ -38,6 +57,16 @@ const validateBlocknoteFieldOrThrow = ( ); } + if (hasDangerousUrl(textValue)) { + throw new CommonQueryRunnerException( + `Dangerous URL protocol in blocknote content for field "${fieldName}"`, + CommonQueryRunnerExceptionCode.INVALID_ARGS_DATA, + { + userFriendlyMessage: msg`Content contains a URL with a dangerous protocol.`, + }, + ); + } + return textValue; }; diff --git a/packages/twenty-shared/src/utils/index.ts b/packages/twenty-shared/src/utils/index.ts index 59e7e4b2fb..1df43b3c7f 100644 --- a/packages/twenty-shared/src/utils/index.ts +++ b/packages/twenty-shared/src/utils/index.ts @@ -193,7 +193,9 @@ export { absoluteUrlSchema } from './url/absoluteUrlSchema'; export { buildSignedPath } from './url/buildSignedPath'; export { ensureAbsoluteUrl } from './url/ensureAbsoluteUrl'; export { getAbsoluteUrlOrThrow } from './url/getAbsoluteUrlOrThrow'; +export { getSafeUrl } from './url/getSafeUrl'; export { getUrlHostnameOrThrow } from './url/getUrlHostnameOrThrow'; +export { isSafeUrl } from './url/isSafeUrl'; export { isValidHostname } from './url/isValidHostname'; export { isValidUrl } from './url/isValidUrl'; export { normalizeUrl } from './url/normalizeUrl'; diff --git a/packages/twenty-shared/src/utils/url/getSafeUrl.ts b/packages/twenty-shared/src/utils/url/getSafeUrl.ts new file mode 100644 index 0000000000..e7b5269728 --- /dev/null +++ b/packages/twenty-shared/src/utils/url/getSafeUrl.ts @@ -0,0 +1,17 @@ +import { isSafeUrl } from './isSafeUrl'; + +export const getSafeUrl = ( + url: string | undefined | null, +): string | undefined => { + if (!url || url.trim().length === 0) { + return undefined; + } + + if (isSafeUrl(url)) { + return url; + } + + const withScheme = `https://${url}`; + + return isSafeUrl(withScheme) ? withScheme : undefined; +}; diff --git a/packages/twenty-shared/src/utils/url/index.ts b/packages/twenty-shared/src/utils/url/index.ts index 1a8ca83304..8ff633915b 100644 --- a/packages/twenty-shared/src/utils/url/index.ts +++ b/packages/twenty-shared/src/utils/url/index.ts @@ -1,4 +1,6 @@ export * from './absoluteUrlSchema'; +export * from './getSafeUrl'; +export * from './isSafeUrl'; export * from './ensureAbsoluteUrl'; export * from './getAbsoluteUrlOrThrow'; export * from './getUrlHostnameOrThrow'; diff --git a/packages/twenty-shared/src/utils/url/isSafeUrl.ts b/packages/twenty-shared/src/utils/url/isSafeUrl.ts new file mode 100644 index 0000000000..c2df3e6957 --- /dev/null +++ b/packages/twenty-shared/src/utils/url/isSafeUrl.ts @@ -0,0 +1,15 @@ +const SAFE_URL_PROTOCOLS = ['http:', 'https:', 'mailto:', 'tel:']; + +export const isSafeUrl = (url: string): boolean => { + if (url.startsWith('/') && !url.startsWith('//')) { + return true; + } + + try { + const parsed = new URL(url); + + return SAFE_URL_PROTOCOLS.includes(parsed.protocol); + } catch { + return false; + } +};