From a92ae9f6839c38d88c7d5adb00b2ae42470bd65f Mon Sep 17 00:00:00 2001 From: Itroun <78351901+Itroun@users.noreply.github.com> Date: Thu, 23 Apr 2026 13:55:48 +0200 Subject: [PATCH] fix: Tighten share-URL validation against malicious color strings `validateShareData` previously only checked `version === 1` and the truthiness of `grid`/`colors`, so a crafted share URL could set `patternColors` to arbitrary strings. Those strings flow into `exportSvg` as `fill="${color}"` via string concatenation, so a payload like `"/>` in a downloaded SVG would execute when the victim opens the file directly in a browser. Strict-validate share data at the boundary: hex-regex each color in `colors.pattern`, `colors.background`, and `palette.custom`; require plain objects and finite numeric dimensions; require array-of-arrays shape for cells. Also run `sanitizeGrid` on share-URL cells to match the file-import path (export.js:817). Promoted `sanitizeGrid` to an exported helper. --- src/core/export.js | 2 +- src/main.js | 6 +++-- src/utils/sharing.js | 45 ++++++++++++++++++++++++++++++++++---- src/utils/validation.js | 10 +++++---- tests/unit/sharing.test.js | 44 +++++++++++++++++++++++++++++++++++++ 5 files changed, 96 insertions(+), 11 deletions(-) diff --git a/src/core/export.js b/src/core/export.js index 7857848..cb05e36 100644 --- a/src/core/export.js +++ b/src/core/export.js @@ -14,7 +14,7 @@ import { Utils } from '../utils.js'; * @param {Array} grid - Grid data from imported file * @returns {Array} Sanitized grid with all null/undefined converted to 0 */ -function sanitizeGrid(grid) { +export function sanitizeGrid(grid) { if (!Array.isArray(grid)) { return grid; } diff --git a/src/main.js b/src/main.js index 30caf39..e03c7e3 100644 --- a/src/main.js +++ b/src/main.js @@ -8,7 +8,7 @@ import { StorageManager } from './managers/storage.js'; import { HistoryManager } from './managers/history.js'; import { CanvasManager } from './managers/canvas.js'; import { createEmptyGrid, resizeGrid, resizeGridFromEdge } from './core/grid.js'; -import { exportSvg, exportPng, exportPreviewSvg, exportPreviewPng, exportPatternWithContextSvg, exportPatternWithContextPng, exportJson, importJson, downloadFile } from './core/export.js'; +import { exportSvg, exportPng, exportPreviewSvg, exportPreviewPng, exportPatternWithContextSvg, exportPatternWithContextPng, exportJson, importJson, downloadFile, sanitizeGrid } from './core/export.js'; import { generateShareUrl, parseShareUrl, copyToClipboard, validateShareData } from './utils/sharing.js'; import { validateGridDimension, @@ -1849,7 +1849,9 @@ if (!shareUrlResult.success) { CONFIG.MAX_ASPECT_RATIO, CONFIG.DEFAULT_ASPECT_RATIO ); - grid = patternData.grid.cells || createEmptyGrid(gridWidth, gridHeight); + grid = patternData.grid.cells + ? sanitizeGrid(patternData.grid.cells) + : createEmptyGrid(gridWidth, gridHeight); backgroundColor = patternData.colors.background || CONFIG.DEFAULT_BACKGROUND_COLOR; patternColors = patternData.colors.pattern || [CONFIG.DEFAULT_PATTERN_COLOR]; activePatternIndex = 0; diff --git a/src/utils/sharing.js b/src/utils/sharing.js index 28a3c62..5ef1b22 100644 --- a/src/utils/sharing.js +++ b/src/utils/sharing.js @@ -6,6 +6,7 @@ import LZString from 'lz-string'; import { exportJson } from '../core/export.js'; +import { HEX_COLOR_PATTERN } from './validation.js'; // URL format constants const SHARE_URL_PREFIX = '#p='; @@ -153,15 +154,51 @@ export async function copyToClipboard(text) { } } +function isHexArray(arr) { + if (!Array.isArray(arr)) return false; + for (const c of arr) { + if (typeof c !== 'string' || !HEX_COLOR_PATTERN.test(c)) return false; + } + return true; +} + /** - * Validate share URL data against schema + * Validate share URL data against schema. Strict: rejects anything whose + * shape or types don't match the export format, so attacker-controlled + * share URLs can't smuggle non-string values into color sinks (SVG export, + * style.backgroundColor) or non-numeric values into the grid. * @param {Object} data - Parsed share data * @returns {boolean} - Whether data is valid */ export function validateShareData(data) { - if (!data || typeof data !== 'object') return false; - if (!data.version || data.version !== 1) return false; - if (!data.grid || !data.colors) return false; + if (!data || typeof data !== 'object' || Array.isArray(data)) return false; + if (data.version !== 1) return false; + + const { grid, colors } = data; + if (!grid || typeof grid !== 'object' || Array.isArray(grid)) return false; + if (!colors || typeof colors !== 'object' || Array.isArray(colors)) return false; + + if (typeof grid.width !== 'number' || !Number.isFinite(grid.width)) return false; + if (typeof grid.height !== 'number' || !Number.isFinite(grid.height)) return false; + if (grid.aspectRatio !== undefined && + (typeof grid.aspectRatio !== 'number' || !Number.isFinite(grid.aspectRatio))) return false; + if (grid.cells !== undefined) { + if (!Array.isArray(grid.cells)) return false; + for (const row of grid.cells) { + if (!Array.isArray(row)) return false; + } + } + + if (typeof colors.background !== 'string' || !HEX_COLOR_PATTERN.test(colors.background)) return false; + if (!isHexArray(colors.pattern) || colors.pattern.length === 0) return false; + + // Optional custom palette flows into patternColors (which reach SVG string + // concatenation), so each entry must also be a hex string. + if (data.palette !== undefined) { + if (!data.palette || typeof data.palette !== 'object' || Array.isArray(data.palette)) return false; + if (data.palette.custom != null && !isHexArray(data.palette.custom)) return false; + } + return true; } diff --git a/src/utils/validation.js b/src/utils/validation.js index b8504cb..fdfdbbe 100644 --- a/src/utils/validation.js +++ b/src/utils/validation.js @@ -125,13 +125,15 @@ export function validatePreviewRepeat(input, fieldName = 'Preview repeat') { * @param {string} input - The color value to validate * @returns {ValidationResult} */ +// Matches #RGB, #RRGGBB, or #RRGGBBAA. Exported so security-sensitive +// callers (e.g. share-URL validation) can reject non-hex strings at the +// boundary without re-declaring the regex. +export const HEX_COLOR_PATTERN = /^#([A-Fa-f0-9]{3}|[A-Fa-f0-9]{6}|[A-Fa-f0-9]{8})$/; + export function validateColor(input) { const trimmed = input.trim(); - // Match #RGB, #RRGGBB, or #RRGGBBAA - const hexPattern = /^#([A-Fa-f0-9]{3}|[A-Fa-f0-9]{6}|[A-Fa-f0-9]{8})$/; - - if (!hexPattern.test(trimmed)) { + if (!HEX_COLOR_PATTERN.test(trimmed)) { return { valid: false, value: CONFIG.DEFAULT_PATTERN_COLOR, diff --git a/tests/unit/sharing.test.js b/tests/unit/sharing.test.js index f1d0c3a..13b7508 100644 --- a/tests/unit/sharing.test.js +++ b/tests/unit/sharing.test.js @@ -172,6 +172,50 @@ describe('Sharing Utilities', () => { expect(validateShareData(123)).toBe(false); expect(validateShareData([])).toBe(false); }); + + it('should reject non-hex colors', () => { + const base = { + version: 1, + grid: { width: 5, height: 5, cells: [[0]] }, + colors: { background: '#ffffff', pattern: ['#000000'] } + }; + expect(validateShareData({ ...base, colors: { background: 'red', pattern: ['#000000'] } })).toBe(false); + expect(validateShareData({ ...base, colors: { background: '#ffffff', pattern: ['"/>'] } })).toBe(false); + expect(validateShareData({ ...base, colors: { background: '#ffffff', pattern: ['#zzz'] } })).toBe(false); + expect(validateShareData({ ...base, colors: { background: '#ffffff', pattern: [] } })).toBe(false); + }); + + it('should reject non-numeric grid dimensions', () => { + const base = { + version: 1, + grid: { width: 5, height: 5, cells: [[0]] }, + colors: { background: '#ffffff', pattern: ['#000000'] } + }; + expect(validateShareData({ ...base, grid: { ...base.grid, width: '5' } })).toBe(false); + expect(validateShareData({ ...base, grid: { ...base.grid, height: NaN } })).toBe(false); + expect(validateShareData({ ...base, grid: { ...base.grid, aspectRatio: 'wide' } })).toBe(false); + }); + + it('should reject malformed grid cells', () => { + const base = { + version: 1, + grid: { width: 5, height: 5 }, + colors: { background: '#ffffff', pattern: ['#000000'] } + }; + expect(validateShareData({ ...base, grid: { ...base.grid, cells: 'oops' } })).toBe(false); + expect(validateShareData({ ...base, grid: { ...base.grid, cells: [1, 2, 3] } })).toBe(false); + }); + + it('should reject custom palette with non-hex entries', () => { + const base = { + version: 1, + grid: { width: 5, height: 5, cells: [[0]] }, + colors: { background: '#ffffff', pattern: ['#000000'] } + }; + expect(validateShareData({ ...base, palette: { custom: ['#abc', 'javascript:alert(1)'] } })).toBe(false); + expect(validateShareData({ ...base, palette: { custom: ['#abc', '#def'] } })).toBe(true); + expect(validateShareData({ ...base, palette: { custom: null } })).toBe(true); + }); }); describe('copyToClipboard()', () => {