diff --git a/CHANGELOG.md b/CHANGELOG.md index f7075b7bf..952a892d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ You can also check the - Statistics page no longer fails when the database contains malformed chart configs; unsafe cube IRIs are filtered out and SPARQL metadata lookups tolerate endpoint failures + - Harden the chart preview page: fix a prototype pollution vulnerability in + the URL hash parser that could be triggered by crafted preview links, and + ignore unrelated `postMessage` traffic instead of feeding it into chart + state migration ## 6.5.5 - 2026-10-26 diff --git a/app/pages/preview.tsx b/app/pages/preview.tsx index 020712082..d5fddf00c 100644 --- a/app/pages/preview.tsx +++ b/app/pages/preview.tsx @@ -17,8 +17,39 @@ import { migrateConfiguratorState } from "@/utils/chart-config/versioning"; import { hashStringToObject } from "@/utils/hash-utils"; import { maybeWindow } from "@/utils/maybe-window"; +// Top-level keys that a posted configurator state may contain. Used to +// distinguish chart state messages from unrelated postMessage traffic +// (browser extensions, React DevTools backend, ...). +const CONFIG_STATE_KEYS = [ + "version", + "state", + "dataSource", + "dataSet", + "chartConfig", + "chartConfigs", + "layout", + "dashboardFilters", + "activeChartKey", + "meta", +] as const; + const isValidMessage = (e: MessageEvent) => { - return e.data && !e.data.type; + const data = e.data; + + // The documented API contract: the embedding parent window posts the + // configurator state object to this iframe. + if ( + data === null || + typeof data !== "object" || + Array.isArray(data) || + e.source !== window.parent + ) { + return false; + } + + return CONFIG_STATE_KEYS.some((key) => + Object.prototype.hasOwnProperty.call(data, key) + ); }; const chartStateStore = create<{ diff --git a/app/utils/hash-utils.spec.ts b/app/utils/hash-utils.spec.ts index 145775eb7..bcdc07de9 100644 --- a/app/utils/hash-utils.spec.ts +++ b/app/utils/hash-utils.spec.ts @@ -149,3 +149,40 @@ it("should persist chart configs as is", () => { expect(configuratorState).toEqual(configuratorStateFromHashString); }); + +it("should not allow prototype pollution via __proto__ path segments", () => { + const hashString = + "#__proto____SEP__pollutedKey=pwned||__proto____SEP__dangerouslySetInnerHTML=%7B%7D||__proto____SEP____html=%3Cimg%20src=x%20onerror=alert(1)%3E"; + + hashStringToObject(hashString); + + expect(({} as any).pollutedKey).toBeUndefined(); + expect(({} as any).dangerouslySetInnerHTML).toBeUndefined(); + expect(({} as any).__html).toBeUndefined(); +}); + +it("should not allow prototype pollution via constructor/prototype path segments", () => { + const hashString = + "#constructor__SEP__prototype__SEP__pollutedKey=pwned||a__SEP__constructor__SEP__prototype__SEP__pollutedKey2=pwned2||b__SEP____proto____SEP__pollutedKey3=pwned3"; + + const result = hashStringToObject(hashString); + + expect(({} as any).pollutedKey).toBeUndefined(); + expect(({} as any).pollutedKey2).toBeUndefined(); + expect(({} as any).pollutedKey3).toBeUndefined(); + expect(result).toEqual({}); +}); + +it("should not traverse inherited object properties when unflattening", () => { + const hashString = + "#hasOwnProperty__SEP__pollutedKey=pwned||valueOf__SEP__pollutedKey2=pwned2||toString__SEP__pollutedKey3=pwned3"; + + const result = hashStringToObject(hashString); + + // The walk must create own properties instead of descending into the + // shared built-in functions found on Object.prototype. + expect(({} as any).pollutedKey).toBeUndefined(); + expect(({} as any).pollutedKey2).toBeUndefined(); + expect(({} as any).pollutedKey3).toBeUndefined(); + expect(result.hasOwnProperty).toBeDefined(); +}); diff --git a/app/utils/hash-utils.ts b/app/utils/hash-utils.ts index d0892bc1a..5bb9c91ee 100644 --- a/app/utils/hash-utils.ts +++ b/app/utils/hash-utils.ts @@ -117,17 +117,34 @@ const parseValue = (v: any) => { return v; }; +// Path segments that must never be traversed or written: they would allow an +// attacker to escape the result object and pollute Object.prototype +// (e.g. "__proto__" resolves to the prototype, "constructor"/"prototype" +// reach it via Object.prototype.constructor.prototype). +const BLOCKED_KEYS = new Set(["__proto__", "constructor", "prototype"]); + +const hasOwn = (o: Object, k: string) => + Object.prototype.hasOwnProperty.call(o, k); + const unflattenObject = (o: Object) => { const result: Object = {}; for (const rootKey in o) { - if (o.hasOwnProperty(rootKey)) { + if (hasOwn(o, rootKey)) { const keys = rootKey.split(KEY_SEPARATOR); + + if (keys.some((k) => BLOCKED_KEYS.has(k))) { + continue; + } + keys.reduce((acc, k, i) => { if (i === keys.length - 1) { acc[k] = o[rootKey]; } else { - if (!(k in acc)) { + // Own-property check: `k in acc` would also match inherited + // properties (Object.prototype members, the shared `constructor` + // function, ...) and let the walk escape `result`. + if (!hasOwn(acc, k)) { acc[k] = {}; } @@ -145,7 +162,7 @@ const unflattenObject = (o: Object) => { } for (const k in obj) { - if (obj.hasOwnProperty(k)) { + if (hasOwn(obj, k)) { obj[k] = transformArraysAndScalars(obj[k]); } }