diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f197913a..f06c257e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ You can also check the ## next release +- Fixes + - Sanitize HTML coming from cube metadata and from WMS / WMTS capabilities + documents instead of injecting it into the DOM. + ## 6.5.2 – 2026-08-04 - Fixes diff --git a/app/.env.development b/app/.env.development index 01156e380..6fd0ca187 100644 --- a/app/.env.development +++ b/app/.env.development @@ -2,7 +2,7 @@ DATABASE_URL=postgres://postgres:password@localhost:5432/visualization_tool ENDPOINT=sparql+https://cached.lindas.admin.ch/query SPARQL_GEO_ENDPOINT=https://geo.ld.admin.ch/query GRAPHQL_ENDPOINT=/api/graphql -WHITELISTED_DATA_SOURCES=["Prod", "Prod-uncached", "Int", "Int-uncached", "Test", "Test-uncached"] +WHITELISTED_DATA_SOURCES='["Prod", "Prod-uncached", "Int", "Int-uncached", "Test", "Test-uncached"]' SENTRY_IGNORE_API_RESOLUTION_ERROR=1 MAPTILER_API_KEY=123 ADFS_PROFILE_URL=https://www.myaccount-r.eiam.admin.ch/ diff --git a/app/browse/ui/dataset-result.tsx b/app/browse/ui/dataset-result.tsx index 607d51598..ef4c7ad80 100644 --- a/app/browse/ui/dataset-result.tsx +++ b/app/browse/ui/dataset-result.tsx @@ -1,5 +1,5 @@ import { Trans } from "@lingui/macro"; -import { Box, CardProps, Stack, Theme, Typography } from "@mui/material"; +import { CardProps, Stack, Theme, Typography } from "@mui/material"; import { makeStyles } from "@mui/styles"; import clsx from "clsx"; import sortBy from "lodash/sortBy"; @@ -12,6 +12,8 @@ import { DateFormat } from "@/browse/ui/date-format"; import { Flex } from "@/components/flex"; import { MaybeTooltip } from "@/components/maybe-tooltip"; import { MotionCard, smoothPresenceProps } from "@/components/presence"; +import { boldOnlySchema } from "@/components/sanitize-schema"; +import { SanitizedHtml } from "@/components/sanitized-html"; import { Tag } from "@/components/tag"; import { PartialSearchCube } from "@/domain/data"; import { DataCubePublicationStatus } from "@/graphql/resolver-types"; @@ -97,11 +99,14 @@ export const DatasetResult = ({ onClick={disableTitleLink ? undefined : handleTitleClick} > {highlightedTitle ? ( - , so the rest of the + // title is rendered with a regular weight. + fontWeight={highlightedTitle.includes("") ? 400 : 700} + html={highlightedTitle} + schema={boldOnlySchema} /> ) : ( title @@ -113,10 +118,11 @@ export const DatasetResult = ({ title={description ?? ""} > {highlightedDescription ? ( - ) : ( description diff --git a/app/charts/map/map-custom-layers-legend.spec.tsx b/app/charts/map/map-custom-layers-legend.spec.tsx new file mode 100644 index 000000000..f39e3ef90 --- /dev/null +++ b/app/charts/map/map-custom-layers-legend.spec.tsx @@ -0,0 +1,78 @@ +import { cleanup, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it } from "vitest"; + +import { CustomLayerDescription } from "@/charts/map/map-custom-layers-legend"; +import { parseWMSContent } from "@/charts/map/wms-utils"; + +afterEach(cleanup); + +// Reproduces an attacker-controlled GetCapabilities document whose +// carries an XSS payload next to legitimate formatting. +const CAPABILITIES = ` + + probe + + + + + + + + + + root + + x-probe-layer + Probe + Provided by Swisstopo, see details.]]> + EPSG:3857 + + + +`; + +describe("CustomLayerDescription", () => { + it("strips dangerous markup from WMS layer descriptions", () => { + const { container } = render( + + ); + + expect(container.querySelector("img")).toBeNull(); + expect(container.innerHTML).not.toContain("onerror"); + }); + + it("keeps the formatting of WMS layer descriptions", () => { + const { container } = render( + + ); + + expect(container.querySelector("b")?.textContent).toBe("Swisstopo"); + + const link = screen.getByRole("link", { name: "details" }); + expect(link.getAttribute("href")).toBe("https://example.com/info"); + expect(link.getAttribute("rel")).toBe("noopener noreferrer"); + }); + + it("renders a hostile WMS abstract without executing it", () => { + const layers = parseWMSContent( + CAPABILITIES, + "https://attacker.example/wms" + ); + const description = layers.find( + (d) => d.id === "x-probe-layer" + )?.description; + + expect(description).toContain("onerror"); + + const { container } = render( + + ); + + expect(container.querySelector("img")).toBeNull(); + expect(container.innerHTML).not.toContain("onerror"); + expect(container.querySelector("b")?.textContent).toBe("Swisstopo"); + expect( + screen.getByRole("link", { name: "details" }).getAttribute("href") + ).toBe("https://example.com/info"); + }); +}); diff --git a/app/charts/map/map-custom-layers-legend.tsx b/app/charts/map/map-custom-layers-legend.tsx index 39cdce5ec..1b548f373 100644 --- a/app/charts/map/map-custom-layers-legend.tsx +++ b/app/charts/map/map-custom-layers-legend.tsx @@ -1,4 +1,4 @@ -import { Box, Typography, useTheme } from "@mui/material"; +import { Box, Typography } from "@mui/material"; import uniq from "lodash/uniq"; import NextImage from "next/image"; @@ -14,6 +14,8 @@ import { } from "@/charts/map/wmts-utils"; import { Error, InlineLoading } from "@/components/hint"; import { InfoIconTooltip } from "@/components/info-icon-tooltip"; +import { inlineTextSchema } from "@/components/sanitize-schema"; +import { SanitizedHtml } from "@/components/sanitized-html"; import { BaseLayer, MapConfig } from "@/config-types"; import { truthy } from "@/domain/types"; import { useLocale } from "@/locales/use-locale"; @@ -43,6 +45,19 @@ const constrainSize = ({ return { width, height }; }; +export const CustomLayerDescription = ({ + description, +}: { + description: string; +}) => ( + +); + export const MapCustomLayersLegend = ({ chartConfig, value, @@ -52,7 +67,6 @@ export const MapCustomLayersLegend = ({ }) => { const customLayers = chartConfig.baseLayer.customLayers; const { data: legendsData, error } = useLegendsData({ customLayers }); - const theme = useTheme(); return error ? ( {error.message} ) : !legendsData ? ( @@ -99,15 +113,7 @@ export const MapCustomLayersLegend = ({ {layer.description ? ( *": { - // We do not let the tooltip HTML override the font size - fontSize: `${theme.typography.caption.fontSize} !important`, - }, - }} - dangerouslySetInnerHTML={{ __html: layer.description }} - /> + } sx={{ width: "fit-content" }} /> diff --git a/app/components/dataset-metadata.spec.tsx b/app/components/dataset-metadata.spec.tsx new file mode 100644 index 000000000..7d1c3f3c6 --- /dev/null +++ b/app/components/dataset-metadata.spec.tsx @@ -0,0 +1,45 @@ +import { cleanup, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it } from "vitest"; + +import { DatasetPublisher } from "@/components/dataset-metadata"; + +afterEach(cleanup); + +describe("DatasetPublisher", () => { + it("renders publisher anchor markup as a safe link", () => { + render( + FOEN & BAFU' + } + /> + ); + + const link = screen.getByRole("link", { name: "FOEN & BAFU" }); + expect(link.getAttribute("href")).toBe("https://example.com/?a=1&b=2"); + expect(link.getAttribute("target")).toBe("_blank"); + expect(link.getAttribute("rel")).toBe("noopener noreferrer"); + }); + + it("does not render unsafe publisher URLs as links", () => { + const { container } = render( + Publisher'} + /> + ); + + expect(screen.getByText("Publisher")).toBeTruthy(); + expect(container.querySelector("a")).toBeNull(); + }); + + it("renders plain text and strips unexpected markup", () => { + const { container } = render( + & Office'} + /> + ); + + expect(container.textContent).toBe("Publisher & Office"); + expect(container.querySelector("img")).toBeNull(); + }); +}); diff --git a/app/components/dataset-metadata.tsx b/app/components/dataset-metadata.tsx index d42c2ab50..abed6d895 100644 --- a/app/components/dataset-metadata.tsx +++ b/app/components/dataset-metadata.tsx @@ -1,7 +1,6 @@ import { sanitizeUrl } from "@braintree/sanitize-url"; import { Trans } from "@lingui/macro"; import { - Box, Link, Link as MUILink, LinkProps, @@ -15,6 +14,8 @@ import { ReactElement, ReactNode } from "react"; import { useQueryFilters } from "@/charts/shared/chart-helpers"; import { DataDownloadMenu } from "@/components/data-download"; +import { inlineTextSchema } from "@/components/sanitize-schema"; +import { SanitizedHtml } from "@/components/sanitized-html"; import { Tag } from "@/components/tag"; import { DataSource } from "@/configurator"; import { DataCubeMetadata } from "@/domain/data"; @@ -54,13 +55,7 @@ export const DatasetMetadata = ({ Source - a": { color: "grey.900" } }} - dangerouslySetInnerHTML={{ - __html: cube.publisher, - }} - /> + )} @@ -173,6 +168,15 @@ const DatasetMetadataBody = ({ ); +export const DatasetPublisher = ({ publisher }: { publisher: string }) => ( + a": { color: "grey.900" } }} + /> +); + const DatasetMetadataLink = ({ href, label, diff --git a/app/components/debug-search.tsx b/app/components/debug-search.tsx index 1ab7d5433..e7d077f6a 100644 --- a/app/components/debug-search.tsx +++ b/app/components/debug-search.tsx @@ -10,10 +10,9 @@ import TextField from "@mui/material/TextField"; import Typography from "@mui/material/Typography"; import { KeyboardEventHandler, useEffect, useRef, useState } from "react"; -import { - SearchCubeFilter, - useSearchCubesQuery, -} from "@/graphql/query-hooks"; +import { boldOnlySchema } from "@/components/sanitize-schema"; +import { SanitizedHtml } from "@/components/sanitized-html"; +import { SearchCubeFilter, useSearchCubesQuery } from "@/graphql/query-hooks"; import { RequestQueryMeta } from "@/graphql/query-meta"; import { SearchCubeFilterType } from "@/graphql/resolver-types"; @@ -126,17 +125,20 @@ const Search = ({ ({ cube, highlightedTitle, highlightedDescription }) => { return (
- - -
+ + + + + + {cube.iri} {cube.themes.map((t) => ( diff --git a/app/components/markdown.tsx b/app/components/markdown.tsx index 7dca5c1fe..0461a7cc4 100644 --- a/app/components/markdown.tsx +++ b/app/components/markdown.tsx @@ -1,44 +1,13 @@ import clsx from "clsx"; import { ComponentProps } from "react"; import ReactMarkdown from "react-markdown"; -import rehypeSanitize, { defaultSchema } from "rehype-sanitize"; +import rehypeSanitize from "rehype-sanitize"; import remarkGfm from "remark-gfm"; import classes from "@/components/markdown.module.css"; +import { richTextSchema } from "@/components/sanitize-schema"; import { palette } from "@/themes/palette"; -const sanitizeSchema = { - ...defaultSchema, - tagNames: [ - "h1", - "h2", - "h3", - "h4", - "h5", - "h6", - "p", - "strong", - "em", - "ins", - "del", - "s", - "ul", - "ol", - "li", - "blockquote", - "code", - "pre", - "br", - "hr", - "a", - ], - attributes: { - ...defaultSchema.attributes, - a: ["href", "title"], - "*": defaultSchema.attributes?.["*"] ?? [], - }, -}; - const components: ComponentProps["components"] = { h1: ({ children, className, ...props }) => (

@@ -93,10 +62,10 @@ export const Markdown = ( ) => { return ( ); }; @@ -109,6 +78,7 @@ export const InlineMarkdown = ({ return ( <>{children}, strong: ({ children }) => {children}, @@ -130,8 +100,8 @@ export const InlineMarkdown = ({ code: ({ children }) => {children}, br: () => <> , }} + rehypePlugins={[[rehypeSanitize, richTextSchema]]} skipHtml - {...rest} > {inlineMarkdown} @@ -215,10 +185,10 @@ export const MarkdownInheritFonts = ( ) => { return ( ); }; diff --git a/app/components/sanitize-schema.ts b/app/components/sanitize-schema.ts new file mode 100644 index 000000000..0f75d4abc --- /dev/null +++ b/app/components/sanitize-schema.ts @@ -0,0 +1,84 @@ +import { defaultSchema, Schema } from "hast-util-sanitize"; + +/** + * Allowlists used to sanitize HTML that we do not control, either because it + * comes from remote cube metadata or because it is fetched at runtime from a + * third-party endpoint configured in the chart. + * + * They all extend `defaultSchema`, which already restricts the protocols + * allowed in `href`/`src` (so e.g. `javascript:` URLs are dropped) and prefixes + * `id`/`name` attributes to avoid DOM clobbering. + */ + +/** Formatting we allow in markdown-authored rich text (see `@/components/markdown`). */ +export const richTextSchema: Schema = { + ...defaultSchema, + tagNames: [ + "h1", + "h2", + "h3", + "h4", + "h5", + "h6", + "p", + "strong", + "em", + "ins", + "del", + "s", + "ul", + "ol", + "li", + "blockquote", + "code", + "pre", + "br", + "hr", + "a", + ], + attributes: { + ...defaultSchema.attributes, + a: ["href", "title"], + "*": defaultSchema.attributes?.["*"] ?? [], + }, +}; + +/** + * Inline formatting, for short snippets rendered inside a single line or + * tooltip (WMS/WMTS layer abstracts, `dcterms:publisher` literals). + */ +export const inlineTextSchema: Schema = { + ...defaultSchema, + tagNames: [ + "a", + "b", + "strong", + "i", + "em", + "u", + "s", + "br", + "p", + "ul", + "ol", + "li", + "code", + "sub", + "sup", + ], + attributes: { + ...defaultSchema.attributes, + a: ["href", "title"], + "*": defaultSchema.attributes?.["*"] ?? [], + }, +}; + +/** + * Search hit highlighting, where `` is the only tag `highlight` emits (see + * `@/rdf/query-search-score-utils`). + */ +export const boldOnlySchema: Schema = { + ...defaultSchema, + tagNames: ["b"], + attributes: {}, +}; diff --git a/app/components/sanitized-html.spec.tsx b/app/components/sanitized-html.spec.tsx new file mode 100644 index 000000000..a3d36a99a --- /dev/null +++ b/app/components/sanitized-html.spec.tsx @@ -0,0 +1,99 @@ +import { cleanup, render, screen } from "@testing-library/react"; +import { renderToStaticMarkup } from "react-dom/server"; +import { afterEach, describe, expect, it } from "vitest"; + +import { boldOnlySchema, inlineTextSchema } from "@/components/sanitize-schema"; +import { SanitizedHtml } from "@/components/sanitized-html"; + +afterEach(cleanup); + +describe("SanitizedHtml", () => { + it("keeps allowed formatting", () => { + const { container } = render( + + ); + + expect(container.querySelector("b")?.textContent).toBe("bold"); + expect(container.querySelector("em")?.textContent).toBe("emphasized"); + }); + + it("drops disallowed elements but keeps their text", () => { + const { container } = render( + alert(1) after'} + schema={inlineTextSchema} + /> + ); + + expect(container.querySelector("script")).toBeNull(); + expect(container.querySelector("iframe")).toBeNull(); + expect(container.textContent).toContain("Before"); + expect(container.textContent).toContain("after"); + }); + + it("drops event handler attributes", () => { + const { container } = render( + '} + schema={inlineTextSchema} + /> + ); + + expect(container.querySelector("img")).toBeNull(); + expect(container.innerHTML).not.toContain("onerror"); + }); + + it("renders links as safe external links", () => { + render( + FOEN & BAFU'} + schema={inlineTextSchema} + /> + ); + + const link = screen.getByRole("link", { name: "FOEN & BAFU" }); + expect(link.getAttribute("href")).toBe("https://example.com/?a=1&b=2"); + expect(link.getAttribute("target")).toBe("_blank"); + expect(link.getAttribute("rel")).toBe("noopener noreferrer"); + }); + + it("does not render unsafe URLs as links", () => { + const { container } = render( + Publisher'} + schema={inlineTextSchema} + /> + ); + + expect(container.querySelector("a")).toBeNull(); + expect(screen.getByText("Publisher")).toBeTruthy(); + }); + + it("restricts the markup to the given schema", () => { + const { container } = render( + match link'} + schema={boldOnlySchema} + /> + ); + + expect(container.querySelector("b")?.textContent).toBe("match"); + expect(container.querySelector("a")).toBeNull(); + expect(container.textContent).toBe("match link"); + }); + + it("renders on the server", () => { + const markup = renderToStaticMarkup( + ok'} + schema={inlineTextSchema} + /> + ); + + expect(markup).toContain("ok"); + expect(markup).not.toContain("onerror"); + }); +}); diff --git a/app/components/sanitized-html.tsx b/app/components/sanitized-html.tsx new file mode 100644 index 000000000..6416bc4bb --- /dev/null +++ b/app/components/sanitized-html.tsx @@ -0,0 +1,53 @@ +import { sanitizeUrl } from "@braintree/sanitize-url"; +import { Box, BoxProps } from "@mui/material"; +import { fromHtml } from "hast-util-from-html"; +import { sanitize, Schema } from "hast-util-sanitize"; +import { Components, toJsxRuntime } from "hast-util-to-jsx-runtime"; +import { useMemo } from "react"; +import { Fragment, jsx, jsxs } from "react/jsx-runtime"; + +import { inlineTextSchema } from "@/components/sanitize-schema"; + +const components: Partial = { + a: ({ children, href, ...props }) => { + // The schema already restricts the allowed protocols; sanitizing the URL + // again keeps the guarantee even if a caller passes a laxer schema. + const safeHref = href ? sanitizeUrl(href) : undefined; + + return safeHref && safeHref !== "about:blank" ? ( + + {children} + + ) : ( + <>{children} + ); + }, +}; + +/** + * Renders HTML that we do not control (remote cube metadata, capabilities + * documents fetched from third-party endpoints) as React elements, keeping the + * formatting but dropping everything that is not in `schema`. + * + * Prefer this over `dangerouslySetInnerHTML`, so that sanitization happens in a + * single place. Remaining props are forwarded to the wrapping `Box`. + */ +export const SanitizedHtml = ({ + html, + schema = inlineTextSchema, + ...boxProps +}: { + html: string; + schema?: Schema; +} & BoxProps) => { + const content = useMemo(() => { + return toJsxRuntime(sanitize(fromHtml(html, { fragment: true }), schema), { + Fragment, + jsx, + jsxs, + components, + }); + }, [html, schema]); + + return {content}; +}; diff --git a/app/package.json b/app/package.json index cf046c6d1..5c4962bea 100644 --- a/app/package.json +++ b/app/package.json @@ -115,6 +115,9 @@ "graphql-constraint-directive": "v2", "graphql-depth-limit": "^1.1.0", "graphql-tag": "^2.12.6", + "hast-util-from-html": "^2.0.3", + "hast-util-sanitize": "^5.0.2", + "hast-util-to-jsx-runtime": "^2.3.2", "html-to-image": "^1.11.11", "iframe-resizer": "^4.2.11", "immer": "^9.0.6", diff --git a/app/rdf/query-search-score-utils.spec.ts b/app/rdf/query-search-score-utils.spec.ts index 90d319412..4962adec0 100644 --- a/app/rdf/query-search-score-utils.spec.ts +++ b/app/rdf/query-search-score-utils.spec.ts @@ -22,4 +22,35 @@ describe("highlighting search words in query", () => { expect(result).toEqual(t[2]); } }); + + it("should escape HTML contained in the text", () => { + expect(highlight(' bad', "bad")).toEqual( + "<img src=x onerror="alert(1)"> bad" + ); + }); + + it("should escape HTML contained in the matched part", () => { + expect(highlight("", "