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>
This commit is contained in:
Rayan Salhab
GitHub
parent
a2418751e6
commit
eb66e03f0e
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue