Fix CVE-2024-54152

Author: edi9999

CHANGELOG.md

@@ -1,3 +1,15 @@
+### 1.4.3
+
+Disallow access to prototype chain (CVE-2024-54152) when using compile with locals (two arguments in the called function) :
+
+```js
+compile("__proto__")({}, {});
+```
+
+=> This now returns undefined, previously it would give you the `__proto__` instance which would allow Remote Code Execution.
+
+Thanks to [@JorianWoltjer](https://github.com/JorianWoltjer) who found the vulnerability and reported it.
+
 ### 1.4.2
 
 Make `handleThis` the default if you use the `Lexer` and `Parser` directly, and you don't use `.compile`.

lib/parse.js

@@ -42,6 +42,17 @@ if ("I".toLowerCase() !== "i") {
 	lowercase = manualLowercase;
 }
 
+// Run a function and disallow temporarly the use of the Function constructor
+// This makes arbitrary code generation attacks way more complicated.
+function runWithFunctionConstructorProtection(fn) {
+	var originalFunctionConstructor = Function.prototype.constructor;
+	delete Function.prototype.constructor;
+	var result = fn();
+	// eslint-disable-next-line no-extend-native
+	Function.prototype.constructor = originalFunctionConstructor;
+	return result;
+}
+
 var jqLite, // delay binding since jQuery could be loaded after us.
 	toString = Object.prototype.toString,
 	getPrototypeOf = Object.getPrototypeOf,
@@ -1690,15 +1701,28 @@ ASTCompiler.prototype = {
 			extra +
 			this.watchFns() +
 			"return fn;";
+
 		// eslint-disable-next-line no-new-func
-		var fn = new Function(
+		var wrappedFn = new Function(
 			"$filter",
 			"getStringValue",
 			"ifDefined",
 			"plus",
 			fnString
 		)(this.$filter, getStringValue, ifDefined, plusFn);
 
+		var fn = function (s, l, a, i) {
+			return runWithFunctionConstructorProtection(function () {
+				return wrappedFn(s, l, a, i);
+			});
+		};
+		fn.assign = function (s, v, l) {
+			return runWithFunctionConstructorProtection(function () {
+				return wrappedFn.assign(s, v, l);
+			});
+		};
+		fn.inputs = wrappedFn.inputs;
+
 		this.state = this.stage = undefined;
 		fn.ast = ast;
 		fn.literal = isLiteral(ast);
@@ -1892,7 +1916,12 @@ ASTCompiler.prototype = {
 						);
 					},
 					intoId &&
-						self.lazyAssign(intoId, self.nonComputedMember("l", ast.name))
+						function () {
+							self.if_(
+								self.hasOwnProperty_("l", ast.name),
+								self.lazyAssign(intoId, self.nonComputedMember("l", ast.name))
+							);
+						}
 				);
 				recursionFn(intoId);
 				break;
@@ -2166,7 +2195,7 @@ ASTCompiler.prototype = {
 	},
 
 	filter: function (filterName) {
-		if (!this.state.filters.hasOwnProperty(filterName)) {
+		if (!hasOwnProperty.call(this.state.filters, filterName)) {
 			this.state.filters[filterName] = this.nextId(true);
 		}
 		return this.state.filters[filterName];
@@ -2258,7 +2287,7 @@ ASTCompiler.prototype = {
 			left +
 			"[" +
 			right +
-			"] : null)"
+			"] : undefined)"
 		);
 	},
 
@@ -2375,7 +2404,7 @@ ASTInterpreter.prototype = {
 		forEach(ast.body, function (expression) {
 			expressions.push(self.recurse(expression.expression));
 		});
-		var fn =
+		var wrappedFn =
 			ast.body.length === 0
 				? noop
 				: ast.body.length === 1
@@ -2389,10 +2418,22 @@ ASTInterpreter.prototype = {
 						};
 
 		if (assign) {
-			fn.assign = function (scope, value, locals) {
+			wrappedFn.assign = function (scope, value, locals) {
 				return assign(scope, locals, value);
 			};
 		}
+
+		var fn = function (scope, locals) {
+			return runWithFunctionConstructorProtection(function () {
+				return wrappedFn(scope, locals);
+			});
+		};
+		fn.assign = function (scope, value, locals) {
+			return runWithFunctionConstructorProtection(function () {
+				return wrappedFn.assign(scope, value, locals);
+			});
+		};
+
 		if (inputs) {
 			fn.inputs = inputs;
 		}
@@ -2720,7 +2761,10 @@ ASTInterpreter.prototype = {
 			if (create && create !== 1 && base && base[name] == null) {
 				base[name] = {};
 			}
-			var value = base ? base[name] : undefined;
+			var value;
+			if (base && hasOwnProperty.call(base, name)) {
+				value = base ? base[name] : undefined;
+			}
 			if (context) {
 				return { context: base, name: name, value: value };
 			}

test/main.test.js

@@ -226,13 +226,40 @@ describe("expressions", function () {
 				expect(result).to.equal(undefined);
 			});
 
+			it("should not leak indirectly with string concatenation with locals", function () {
+				evaluate = compile(
+					"a = null; a = ''['c'+'onstructor']['c'+'onstructor']; a = a('return process;'); a();",
+					{ csp: true }
+				);
+				const result = evaluate({}, {});
+				expect(result).to.equal(undefined);
+			});
+
 			it("should not leak indirectly with literal string", function () {
 				evaluate = compile(
 					"a = null; a = ''['constructor']['constructor']; a = a('return process;'); a();"
 				);
 				const result = evaluate({});
 				expect(result).to.equal(undefined);
 			});
+
+			it("should not be able to rewrite hasOwnProperty", function () {
+				const scope = {
+					// Pre-condition: any function in scope that returns a truthy value
+					func: function () {
+						return "anything truthy";
+					},
+				};
+				const options = {
+					// Force to use ASTInterpreter
+					csp: true,
+				};
+				const result = expressions.compile(
+					"hasOwnProperty = func; constructor.getPrototypeOf(toString).constructor('return process')()",
+					options
+				)(scope, scope);
+				expect(result).to.equal(undefined);
+			});
 		});
 
 		describe("when evaluating dot-notated assignments", function () {
@@ -676,6 +703,13 @@ describe("expressions", function () {
 			});
 			expect(evaluate(scope)).to.equal("myval");
 		});
+
+		it("should be possible to calc this+this+this", function () {
+			const evaluate = compile("this+this+this", {
+				csp: false,
+			});
+			expect(evaluate(1)).to.equal(3);
+		});
 	});
 
 	describe("Equality", function () {
@@ -756,7 +790,7 @@ describe("expressions", function () {
 
 		it("should not leak with computed prop", function () {
 			evaluate = compile("a['split']");
-			expect(evaluate({ a: "" })).to.eql(null);
+			expect(evaluate({ a: "" })).to.eql(undefined);
 		});
 
 		it("should allow to read string length", function () {
@@ -782,11 +816,44 @@ describe("expressions", function () {
 			);
 		});
 
-		it("should work with __proto__", function () {
+		it("should not show value of __proto__", function () {
 			evaluate = compile("__proto__");
 			expect(evaluate({})).to.eql(undefined);
 		});
 
+		it("should not show value of __proto__ if passing context (second argument) with csp = false", function () {
+			evaluate = compile("__proto__");
+			expect(evaluate({}, {})).to.eql(undefined);
+		});
+
+		it("should not show value of __proto__ if passing context (second argument) with csp = true", function () {
+			evaluate = compile("__proto__", {
+				csp: true,
+			});
+			expect(evaluate({}, {})).to.eql(undefined);
+		});
+
+		it("should not show value of constructor if passing context (second argument) with csp = true", function () {
+			evaluate = compile("constructor", {
+				csp: true,
+			});
+			expect(evaluate({}, {})).to.eql(undefined);
+		});
+
+		it("should not show value of this['__proto__'] if passing context (second argument) with csp = true", function () {
+			evaluate = compile("this['__proto' + '__']", {
+				csp: true,
+			});
+			expect(evaluate({}, {})).to.eql(undefined);
+		});
+
+		it("should not show value of this['__proto__'] if passing context (second argument) with csp = false", function () {
+			evaluate = compile("this['__proto' + '__']", {
+				csp: false,
+			});
+			expect(evaluate({}, {})).to.eql(undefined);
+		});
+
 		it("should work with toString", function () {
 			evaluate = compile("toString");
 			expect(evaluate({ toString: 10 })).to.eql(10);