From eb66e03f0e67c3b04c863c2b5540c65495bf621d Mon Sep 17 00:00:00 2001 From: Rayan Salhab Date: Sun, 29 Mar 2026 19:27:05 +0300 Subject: [PATCH] fix(security): replace new Function() with safe math expression parser (#753) * fix(security): replace new Function() with safe math expression parser Replace the dangerous new Function() constructor in evaluateMathExpression() with a recursive descent parser that safely evaluates basic arithmetic expressions. The Function constructor is a security anti-pattern that could enable arbitrary code execution if the validation regex is ever bypassed or relaxed. The new parser: - Only supports numbers, +, -, *, /, parentheses, and whitespace - Has no eval-like functionality that could execute arbitrary code - Maintains backward compatibility with existing expressions - Handles operator precedence and parentheses correctly Fixes #725 * Apply suggestion from @coderabbitai[bot] Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: Maze <167211895+mazeincoding@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- apps/web/src/utils/__tests__/math.test.ts | 120 +++++++ apps/web/src/utils/math.ts | 391 +++++++++++++++------- 2 files changed, 396 insertions(+), 115 deletions(-) create mode 100644 apps/web/src/utils/__tests__/math.test.ts diff --git a/apps/web/src/utils/__tests__/math.test.ts b/apps/web/src/utils/__tests__/math.test.ts new file mode 100644 index 00000000..4bdb959f --- /dev/null +++ b/apps/web/src/utils/__tests__/math.test.ts @@ -0,0 +1,120 @@ +import { describe, expect, it } from "bun:test"; +import { evaluateMathExpression } from "../math"; + +describe("evaluateMathExpression", () => { + describe("basic arithmetic", () => { + it("should add numbers", () => { + expect(evaluateMathExpression({ input: "1+2" })).toBe(3); + expect(evaluateMathExpression({ input: "10 + 20" })).toBe(30); + }); + + it("should subtract numbers", () => { + expect(evaluateMathExpression({ input: "5-3" })).toBe(2); + expect(evaluateMathExpression({ input: "100 - 50" })).toBe(50); + }); + + it("should multiply numbers", () => { + expect(evaluateMathExpression({ input: "4*5" })).toBe(20); + expect(evaluateMathExpression({ input: "10 * 10" })).toBe(100); + }); + + it("should divide numbers", () => { + expect(evaluateMathExpression({ input: "20/4" })).toBe(5); + expect(evaluateMathExpression({ input: "100 / 10" })).toBe(10); + }); + }); + + describe("operator precedence", () => { + it("should respect multiplication over addition", () => { + expect(evaluateMathExpression({ input: "2+3*4" })).toBe(14); + expect(evaluateMathExpression({ input: "10+5*2" })).toBe(20); + }); + + it("should respect division over subtraction", () => { + expect(evaluateMathExpression({ input: "10-6/2" })).toBe(7); + }); + }); + + describe("parentheses", () => { + it("should evaluate expressions in parentheses first", () => { + expect(evaluateMathExpression({ input: "(2+3)*4" })).toBe(20); + expect(evaluateMathExpression({ input: "(10-5)*2" })).toBe(10); + }); + + it("should handle nested parentheses", () => { + expect(evaluateMathExpression({ input: "((2+3)*4)" })).toBe(20); + expect(evaluateMathExpression({ input: "(1+(2*3))" })).toBe(7); + }); + }); + + describe("decimal numbers", () => { + it("should handle decimal numbers", () => { + expect(evaluateMathExpression({ input: "3.5+2.5" })).toBe(6); + expect(evaluateMathExpression({ input: "10.5 * 2" })).toBe(21); + }); + }); + + describe("negative numbers", () => { + it("should handle negative numbers", () => { + expect(evaluateMathExpression({ input: "-5+3" })).toBe(-2); + expect(evaluateMathExpression({ input: "10+-5" })).toBe(5); + }); + }); + + describe("whitespace handling", () => { + it("should handle various whitespace", () => { + expect(evaluateMathExpression({ input: " 1 + 2 " })).toBe(3); + expect(evaluateMathExpression({ input: "10* 5" })).toBe(50); + }); + }); + + describe("edge cases", () => { + it("should handle single number", () => { + expect(evaluateMathExpression({ input: "42" })).toBe(42); + expect(evaluateMathExpression({ input: "3.14" })).toBe(3.14); + }); + + it("should return null for empty string", () => { + expect(evaluateMathExpression({ input: "" })).toBeNull(); + }); + + it("should return null for invalid characters", () => { + expect(evaluateMathExpression({ input: "1+2a" })).toBeNull(); + expect(evaluateMathExpression({ input: "alert(1)" })).toBeNull(); + expect(evaluateMathExpression({ input: "1+2; doSomething()" })).toBeNull(); + }); + + it("should return null for unbalanced parentheses", () => { + expect(evaluateMathExpression({ input: "(1+2" })).toBeNull(); + expect(evaluateMathExpression({ input: "1+2)" })).toBeNull(); + }); + + it("should return null for division by zero", () => { + expect(evaluateMathExpression({ input: "1/0" })).toBeNull(); + }); + + it("should return null for incomplete expressions", () => { + expect(evaluateMathExpression({ input: "1+" })).toBeNull(); + expect(evaluateMathExpression({ input: "*2" })).toBeNull(); + }); + }); + + describe("security - code injection prevention", () => { + it("should reject code injection attempts", () => { + // These would be dangerous with new Function() but are safe here + expect(evaluateMathExpression({ input: "alert('xss')" })).toBeNull(); + expect(evaluateMathExpression({ input: "process.exit()" })).toBeNull(); + expect(evaluateMathExpression({ input: "require('fs')" })).toBeNull(); + expect(evaluateMathExpression({ input: "eval('1+1')" })).toBeNull(); + expect(evaluateMathExpression({ input: "Function('return 1')()" })).toBeNull(); + expect(evaluateMathExpression({ input: "constructor.constructor('return 1')()" })).toBeNull(); + expect(evaluateMathExpression({ input: "this.toString" })).toBeNull(); + expect(evaluateMathExpression({ input: "__proto__" })).toBeNull(); + }); + + it("should reject prototype pollution attempts", () => { + expect(evaluateMathExpression({ input: "[].constructor" })).toBeNull(); + expect(evaluateMathExpression({ input: "({}).constructor" })).toBeNull(); + }); + }); +}); diff --git a/apps/web/src/utils/math.ts b/apps/web/src/utils/math.ts index 8043871f..45196b02 100644 --- a/apps/web/src/utils/math.ts +++ b/apps/web/src/utils/math.ts @@ -1,115 +1,276 @@ -export function clamp({ - value, - min, - max, -}: { - value: number; - min: number; - max: number; -}): number { - return Math.max(min, Math.min(max, value)); -} - -export function clampRound({ - value, - min, - max, -}: { - value: number; - min: number; - max: number; -}): number { - return Math.round(clamp({ value, min, max })); -} - -export function getFractionDigitsForStep({ step }: { step: number }): number { - const normalizedStep = step.toString().toLowerCase(); - if (normalizedStep.includes("e-")) { - return Number(normalizedStep.split("e-")[1] ?? 0); - } - const [, fractionalPart = ""] = normalizedStep.split("."); - return fractionalPart.length; -} - -export function snapToStep({ - value, - step, -}: { - value: number; - step: number; -}): number { - if (step <= 0) return value; - const snappedValue = Math.round(value / step) * step; - return Number( - snappedValue.toFixed(getFractionDigitsForStep({ step })), - ); -} - -export function isNearlyEqual({ - leftValue, - rightValue, - epsilon = 0.0001, -}: { - leftValue: number; - rightValue: number; - epsilon?: number; -}): boolean { - return Math.abs(leftValue - rightValue) <= epsilon; -} - -export function formatNumberForDisplay({ - value, - fractionDigits, - minFractionDigits = 0, - maxFractionDigits = 6, -}: { - value: number; - fractionDigits?: number; - minFractionDigits?: number; - maxFractionDigits?: number; -}): string { - const resolvedMaxFractionDigits = Math.max( - 0, - fractionDigits ?? maxFractionDigits, - ); - const resolvedMinFractionDigits = Math.min( - Math.max(0, fractionDigits ?? minFractionDigits), - resolvedMaxFractionDigits, - ); - const fixedValue = value.toFixed(resolvedMaxFractionDigits); - - if (resolvedMaxFractionDigits === 0) { - return Number(fixedValue) === 0 ? "0" : fixedValue; - } - - const [integerPart, fractionPart = ""] = fixedValue.split("."); - const normalizedIntegerPart = Number(fixedValue) === 0 ? "0" : integerPart; - let trimmedFractionPart = fractionPart; - - while ( - trimmedFractionPart.length > resolvedMinFractionDigits && - trimmedFractionPart.endsWith("0") - ) { - trimmedFractionPart = trimmedFractionPart.slice(0, -1); - } - - return trimmedFractionPart - ? `${normalizedIntegerPart}.${trimmedFractionPart}` - : normalizedIntegerPart; -} - -export function evaluateMathExpression({ - input, -}: { - input: string; -}): number | null { - const sanitized = input.trim(); - if (!/^[\d.\s+\-*/()]+$/.test(sanitized)) return null; - try { - const result = new Function(`return (${sanitized})`)(); - if (typeof result !== "number" || !Number.isFinite(result)) return null; - return result; - } catch { - return null; - } -} +export function clamp({ + value, + min, + max, +}: { + value: number; + min: number; + max: number; +}): number { + return Math.max(min, Math.min(max, value)); +} + +export function clampRound({ + value, + min, + max, +}: { + value: number; + min: number; + max: number; +}): number { + return Math.round(clamp({ value, min, max })); +} + +export function getFractionDigitsForStep({ step }: { step: number }): number { + const normalizedStep = step.toString().toLowerCase(); + if (normalizedStep.includes("e-")) { + return Number(normalizedStep.split("e-")[1] ?? 0); + } + const [, fractionalPart = ""] = normalizedStep.split("."); + return fractionalPart.length; +} + +export function snapToStep({ + value, + step, +}: { + value: number; + step: number; +}): number { + if (step <= 0) return value; + const snappedValue = Math.round(value / step) * step; + return Number( + snappedValue.toFixed(getFractionDigitsForStep({ step })), + ); +} + +export function isNearlyEqual({ + leftValue, + rightValue, + epsilon = 0.0001, +}: { + leftValue: number; + rightValue: number; + epsilon?: number; +}): boolean { + return Math.abs(leftValue - rightValue) <= epsilon; +} + +export function formatNumberForDisplay({ + value, + fractionDigits, + minFractionDigits = 0, + maxFractionDigits = 6, +}: { + value: number; + fractionDigits?: number; + minFractionDigits?: number; + maxFractionDigits?: number; +}): string { + const resolvedMaxFractionDigits = Math.max( + 0, + fractionDigits ?? maxFractionDigits, + ); + const resolvedMinFractionDigits = Math.min( + Math.max(0, fractionDigits ?? minFractionDigits), + resolvedMaxFractionDigits, + ); + const fixedValue = value.toFixed(resolvedMaxFractionDigits); + + if (resolvedMaxFractionDigits === 0) { + return Number(fixedValue) === 0 ? "0" : fixedValue; + } + + const [integerPart, fractionPart = ""] = fixedValue.split("."); + const normalizedIntegerPart = Number(fixedValue) === 0 ? "0" : integerPart; + let trimmedFractionPart = fractionPart; + + while ( + trimmedFractionPart.length > resolvedMinFractionDigits && + trimmedFractionPart.endsWith("0") + ) { + trimmedFractionPart = trimmedFractionPart.slice(0, -1); + } + + return trimmedFractionPart + ? `${normalizedIntegerPart}.${trimmedFractionPart}` + : normalizedIntegerPart; +} + +/** + * Safely evaluates a mathematical expression without using eval() or new Function(). + * Supports +, -, *, /, parentheses, and decimal numbers. + */ +export function evaluateMathExpression({ + input, +}: { + input: string; +}): number | null { + const sanitized = input.trim(); + if (!/^[\d.\s+\-*/()]+$/.test(sanitized)) return null; + try { + return parseExpression(sanitized); + } catch { + return null; + } +} + +/** + * Recursive descent parser for basic arithmetic expressions. + * Grammar: + * expression = term { ('+' | '-') term } + * term = factor { ('*' | '/') factor } + * factor = number | '(' expression ')' + */ +function parseExpression(input: string): number | null { + const tokens = tokenize(input); + if (tokens.length === 0) return null; + + let index = 0; + + function peek(): Token | null { + return tokens[index] ?? null; + } + + function consume(): Token | null { + return tokens[index++] ?? null; + } + + function parseExpressionLevel(): number { + let left = parseTerm(); + while (true) { + const token = peek(); + if (token?.type === "PLUS") { + consume(); + left = left + parseTerm(); + } else if (token?.type === "MINUS") { + consume(); + left = left - parseTerm(); + } else { + break; + } + } + return left; + } + + function parseTerm(): number { + let left = parseFactor(); + while (true) { + const token = peek(); + if (token?.type === "MULTIPLY") { + consume(); + left = left * parseFactor(); + } else if (token?.type === "DIVIDE") { + consume(); + const right = parseFactor(); + if (right === 0) throw new Error("Division by zero"); + left = left / right; + } else { + break; + } + } + return left; + } + + function parseFactor(): number { + const token = peek(); + if (!token) throw new Error("Unexpected end of expression"); + + if (token.type === "NUMBER") { + consume(); + return token.value; + } + + if (token.type === "MINUS") { + consume(); + return -parseFactor(); + } + + if (token.type === "LPAREN") { + consume(); + const value = parseExpressionLevel(); + const next = peek(); + if (next?.type !== "RPAREN") { + throw new Error("Missing closing parenthesis"); + } + consume(); + return value; + } + + throw new Error(`Unexpected token: ${token.type}`); + } + + const result = parseExpressionLevel(); + + if (index !== tokens.length) { + return null; + } + + if (!Number.isFinite(result)) return null; + return result; +} + +type Token = + | { type: "NUMBER"; value: number } + | { type: "PLUS" } + | { type: "MINUS" } + | { type: "MULTIPLY" } + | { type: "DIVIDE" } + | { type: "LPAREN" } + | { type: "RPAREN" }; + +function tokenize(input: string): Token[] { + const tokens: Token[] = []; + let i = 0; + const str = input.trim(); + + while (i < str.length) { + const char = str[i]; + + if (/\s/.test(char)) { + i++; + continue; + } + + if (/\d/.test(char)) { + let numStr = ""; + while (i < str.length && (/\d/.test(str[i]) || str[i] === ".")) { + numStr += str[i]; + i++; + } + const value = Number(numStr); + if (!Number.isFinite(value)) { + throw new Error(`Invalid number: ${numStr}`); + } + tokens.push({ type: "NUMBER", value }); + continue; + } + + switch (char) { + case "+": + tokens.push({ type: "PLUS" }); + break; + case "-": + tokens.push({ type: "MINUS" }); + break; + case "*": + tokens.push({ type: "MULTIPLY" }); + break; + case "/": + tokens.push({ type: "DIVIDE" }); + break; + case "(": + tokens.push({ type: "LPAREN" }); + break; + case ")": + tokens.push({ type: "RPAREN" }); + break; + default: + throw new Error(`Invalid character: ${char}`); + } + i++; + } + + return tokens; +}