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
This commit is contained in:
Rayan Salhab
parent
f2eed05d36
commit
0c7cc11392
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -22,6 +22,10 @@ export function isNearlyEqual({
|
|||
return Math.abs(leftValue - rightValue) <= epsilon;
|
||||
}
|
||||
|
||||
/**
|
||||
* Safely evaluates a mathematical expression without using eval() or new Function().
|
||||
* Supports +, -, *, /, parentheses, and decimal numbers.
|
||||
*/
|
||||
export function evaluateMathExpression({
|
||||
input,
|
||||
}: {
|
||||
|
|
@ -30,10 +34,171 @@ export function evaluateMathExpression({
|
|||
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;
|
||||
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();
|
||||
|
||||
// Ensure all tokens were consumed
|
||||
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];
|
||||
|
||||
// Skip whitespace
|
||||
if (/\s/.test(char)) {
|
||||
i++;
|
||||
continue;
|
||||
}
|
||||
|
||||
// Numbers (including decimals)
|
||||
if (/\d/.test(char)) {
|
||||
let numStr = "";
|
||||
while (i < str.length && (/\d/.test(str[i]) || str[i] === ".")) {
|
||||
numStr += str[i];
|
||||
i++;
|
||||
}
|
||||
const value = Number.parseFloat(numStr);
|
||||
if (Number.isNaN(value)) {
|
||||
throw new Error(`Invalid number: ${numStr}`);
|
||||
}
|
||||
tokens.push({ type: "NUMBER", value });
|
||||
continue;
|
||||
}
|
||||
|
||||
// Operators
|
||||
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