From b9a030d02a579b31ada6670a82acc03d308d35fc Mon Sep 17 00:00:00 2001 From: eward Date: Wed, 6 May 2026 14:54:07 +0800 Subject: [PATCH 1/3] fix: guard set paths against prototype pollution --- src/compile/set.ts | 132 +++++++++++++++++++++++++++++++++------------ test/set.test.ts | 87 ++++++++++++++++++++++++++++++ 2 files changed, 184 insertions(+), 35 deletions(-) diff --git a/src/compile/set.ts b/src/compile/set.ts index 1cde584..18e5da7 100644 --- a/src/compile/set.ts +++ b/src/compile/set.ts @@ -1,6 +1,26 @@ -import type { SetAST, VELOCITY_AST } from '../type'; +import type { Attribute, ReferencesAST, SetAST, VELOCITY_AST } from '../type'; import { applyMixins } from '../utils'; import { Compile } from './base-compile'; + +const PROTO_KEY = '__proto__'; +const PROTOTYPE_CHAIN_KEYS = new Set(['constructor', 'prototype']); + +type SetPathTarget = { + blocked: boolean; + rootRef?: unknown; + shouldSetRoot?: boolean; + baseRef?: unknown; + key?: string; +}; + +function hasOwnProperty(baseRef: unknown, key: string): boolean { + return ( + (typeof baseRef === 'object' || typeof baseRef === 'function') && + baseRef !== null && + Object.prototype.hasOwnProperty.call(baseRef, key) + ); +} + /** * #set value */ @@ -20,6 +40,71 @@ export class SetValue extends Compile { // not find local variable, return global context return this.context; } + private getPathKey(exp: Attribute): string { + if (exp.type === 'property') { + return exp.id; + } + + if (exp.type === 'index' && exp.id) { + return String(this.getLiteral(exp.id as VELOCITY_AST)); + } + + return ''; + } + + private isBlockedPathKey(baseRef: unknown, key: string, isEnd: boolean): boolean { + if (key === PROTO_KEY) { + return true; + } + + if (key === 'prototype' && typeof baseRef === 'function') { + return true; + } + + return !isEnd && PROTOTYPE_CHAIN_KEYS.has(key) && !hasOwnProperty(baseRef, key); + } + + private resolveSetPath(context: object, ref: ReferencesAST): SetPathTarget { + if (!ref.path) { + return { blocked: ref.id === PROTO_KEY }; + } + + if (ref.id === PROTO_KEY) { + return { blocked: true }; + } + + let rootRef = (context as Record)[ref.id]; + let shouldSetRoot = false; + + if (typeof rootRef !== 'object') { + rootRef = {}; + shouldSetRoot = true; + } + + let baseRef = rootRef; + + for (let i = 0; i < ref.path.length; i++) { + if (baseRef === undefined || baseRef === null) { + return { blocked: false, rootRef, shouldSetRoot }; + } + + const key = this.getPathKey(ref.path[i]); + const isEnd = ref.path.length === i + 1; + + if (this.isBlockedPathKey(baseRef, key, isEnd)) { + return { blocked: true }; + } + + if (isEnd) { + return { blocked: false, rootRef, shouldSetRoot, baseRef, key }; + } + + baseRef = (baseRef as Record)[key]; + } + + return { blocked: false, rootRef, shouldSetRoot }; + } + /** * parse #set */ @@ -33,6 +118,12 @@ export class SetValue extends Compile { // fix #129 } + const setPath = this.resolveSetPath(context, ref); + + if (setPath.blocked) { + return; + } + const valAst = ast.equal[1]; // eslint-disable-next-line @typescript-eslint/no-explicit-any let val: any; @@ -48,42 +139,13 @@ export class SetValue extends Compile { return; } - let baseRef = (context as Record)[ref.id]; - if (typeof baseRef !== 'object') { - baseRef = {}; + if (setPath.shouldSetRoot) { + (context as Record)[ref.id] = setPath.rootRef; } - (context as Record)[ref.id] = baseRef; - const len = ref.path ? ref.path.length : 0; - - ref.path.some((exp, i) => { - const isEnd = len === i + 1; - let key: string; - - if (exp.type === 'property') { - key = exp.id; - } else if (exp.type === 'index' && exp.id) { - key = String(this.getLiteral(exp.id as VELOCITY_AST)); - } else { - key = ''; - } - - if (isEnd) { - (baseRef as Record)[key] = val; - return true; - } - - baseRef = (baseRef as Record)[key] as Record; - - // such as - // #set($a.d.c2 = 2) - // but $a.d is undefined, value set fail - if (baseRef === undefined) { - return true; - } - - return false; - }); + if (setPath.key !== undefined) { + (setPath.baseRef as Record)[setPath.key] = val; + } } } diff --git a/test/set.test.ts b/test/set.test.ts index 866d8d6..eb2f448 100644 --- a/test/set.test.ts +++ b/test/set.test.ts @@ -190,6 +190,93 @@ describe('Set && Expression', function () { }); }); + describe('prototype pollution protection', function () { + afterEach(function () { + delete (Object.prototype as Record).polluted; + delete (Object.prototype as Record).isAdmin; + }); + + it('does not set values through __proto__ references', function () { + render('#set($__proto__.polluted = "hacked")', {}); + + assert.equal(({} as Record).polluted, undefined); + }); + + it('does not set values through __proto__ index keys', function () { + render('#set($target["__proto__"].polluted = "hacked")', { target: {} }); + + assert.equal(({} as Record).polluted, undefined); + }); + + it('does not set values through inherited constructor.prototype paths', function () { + render('#set($target.constructor.prototype.isAdmin = true)', { target: {} }); + + assert.equal(({} as Record).isAdmin, undefined); + }); + + it('does not evaluate assigned values for blocked dynamic index keys', function () { + let evaluated = false; + + render('#set($key = "__proto__") #set($target[$key].polluted = $markEvaluated())', { + target: {}, + markEvaluated() { + evaluated = true; + return 'hacked'; + }, + }); + + assert.equal(evaluated, false); + assert.equal(({} as Record).polluted, undefined); + }); + + it('does not create root objects for blocked paths', function () { + const context = getContext('#set($target["__proto__"].polluted = "hacked")'); + + expect(context).not.toHaveProperty('target'); + assert.equal(({} as Record).polluted, undefined); + }); + + it('preserves top-level constructor and prototype variables', function () { + const context = getContext('#set($constructor = "ctor") #set($prototype = "proto")'); + + assert.equal(context.constructor, 'ctor'); + assert.equal(context.prototype, 'proto'); + }); + + it('preserves own constructor and prototype data fields', function () { + const model = { + constructor: { + prototype: {}, + }, + prototype: {}, + }; + + render( + '#set($model.constructor.name = "Car") #set($model.constructor.prototype.kind = "vehicle") #set($model.prototype.label = "draft")', + { model } + ); + + assert.equal((model.constructor as Record).name, 'Car'); + assert.equal((model.constructor.prototype as Record).kind, 'vehicle'); + assert.equal((model.prototype as Record).label, 'draft'); + }); + + it('does not set values through function prototype paths', function () { + let evaluated = false; + + render('#set($target.constructor.prototype.polluted = $markEvaluated())', { + target: { constructor: Object }, + markEvaluated() { + evaluated = true; + return 'hacked'; + }, + }); + + assert.equal(evaluated, false); + assert.equal(({} as Record).polluted, undefined); + }); + }); + it('set with foreach', function () { const tpl = ` #foreach($item in [1..2]) From 6c7e6f0d0540a94112cb90ea2e31360889fab109 Mon Sep 17 00:00:00 2001 From: eward Date: Wed, 6 May 2026 15:22:54 +0800 Subject: [PATCH 2/3] fix: preserve set assignments after rhs side effects --- src/compile/set.ts | 32 +++++++++++++++++++++++++------- test/set.test.ts | 15 +++++++++++++++ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/compile/set.ts b/src/compile/set.ts index 18e5da7..ec88798 100644 --- a/src/compile/set.ts +++ b/src/compile/set.ts @@ -52,6 +52,10 @@ export class SetValue extends Compile { return ''; } + private getPathKeys(ref: ReferencesAST): string[] { + return (ref.path || []).map((exp) => this.getPathKey(exp)); + } + private isBlockedPathKey(baseRef: unknown, key: string, isEnd: boolean): boolean { if (key === PROTO_KEY) { return true; @@ -64,8 +68,15 @@ export class SetValue extends Compile { return !isEnd && PROTOTYPE_CHAIN_KEYS.has(key) && !hasOwnProperty(baseRef, key); } - private resolveSetPath(context: object, ref: ReferencesAST): SetPathTarget { - if (!ref.path) { + private hasBlockedUnknownPath(keys: string[], startIndex: number): boolean { + return keys.slice(startIndex).some((key, i) => { + const isEnd = startIndex + i === keys.length - 1; + return key === PROTO_KEY || (!isEnd && PROTOTYPE_CHAIN_KEYS.has(key)); + }); + } + + private resolveSetPath(context: object, ref: ReferencesAST, pathKeys: string[]): SetPathTarget { + if (pathKeys.length === 0) { return { blocked: ref.id === PROTO_KEY }; } @@ -83,13 +94,13 @@ export class SetValue extends Compile { let baseRef = rootRef; - for (let i = 0; i < ref.path.length; i++) { + for (let i = 0; i < pathKeys.length; i++) { if (baseRef === undefined || baseRef === null) { - return { blocked: false, rootRef, shouldSetRoot }; + return { blocked: this.hasBlockedUnknownPath(pathKeys, i), rootRef, shouldSetRoot }; } - const key = this.getPathKey(ref.path[i]); - const isEnd = ref.path.length === i + 1; + const key = pathKeys[i]; + const isEnd = pathKeys.length === i + 1; if (this.isBlockedPathKey(baseRef, key, isEnd)) { return { blocked: true }; @@ -118,7 +129,8 @@ export class SetValue extends Compile { // fix #129 } - const setPath = this.resolveSetPath(context, ref); + const pathKeys = this.getPathKeys(ref); + let setPath = this.resolveSetPath(context, ref, pathKeys); if (setPath.blocked) { return; @@ -139,6 +151,12 @@ export class SetValue extends Compile { return; } + setPath = this.resolveSetPath(context, ref, pathKeys); + + if (setPath.blocked) { + return; + } + if (setPath.shouldSetRoot) { (context as Record)[ref.id] = setPath.rootRef; } diff --git a/test/set.test.ts b/test/set.test.ts index eb2f448..1e3c919 100644 --- a/test/set.test.ts +++ b/test/set.test.ts @@ -236,6 +236,21 @@ describe('Set && Expression', function () { assert.equal(({} as Record).polluted, undefined); }); + it('preserves assignments after assigned values create missing parents', function () { + const target: { child?: { name?: string } } = {}; + const context = { + target, + ensureChild() { + target.child = {}; + return 'Car'; + }, + }; + + render('#set($target.child.name = $ensureChild())', context); + + assert.equal(target.child?.name, 'Car'); + }); + it('preserves top-level constructor and prototype variables', function () { const context = getContext('#set($constructor = "ctor") #set($prototype = "proto")'); From 221d197e1b77564cc5661d69cedce5b321bae940 Mon Sep 17 00:00:00 2001 From: eward Date: Wed, 6 May 2026 15:29:05 +0800 Subject: [PATCH 3/3] docs: explain set path prototype guards --- src/compile/set.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/compile/set.ts b/src/compile/set.ts index ec88798..f29673f 100644 --- a/src/compile/set.ts +++ b/src/compile/set.ts @@ -61,14 +61,22 @@ export class SetValue extends Compile { return true; } + // Function.prototype is a shared prototype object; assigning through it can + // affect every object created by that constructor. if (key === 'prototype' && typeof baseRef === 'function') { return true; } + // `constructor` and `prototype` are valid own data fields. They are only + // dangerous when traversal would fall through to inherited prototype-chain + // properties such as Object.constructor.prototype. return !isEnd && PROTOTYPE_CHAIN_KEYS.has(key) && !hasOwnProperty(baseRef, key); } private hasBlockedUnknownPath(keys: string[], startIndex: number): boolean { + // If an earlier safe segment is missing, the RHS may create it. Before + // evaluating RHS we can only allow the path when the remaining unresolved + // suffix does not contain keys that could later traverse prototypes. return keys.slice(startIndex).some((key, i) => { const isEnd = startIndex + i === keys.length - 1; return key === PROTO_KEY || (!isEnd && PROTOTYPE_CHAIN_KEYS.has(key)); @@ -129,6 +137,8 @@ export class SetValue extends Compile { // fix #129 } + // Resolve the left-hand path before evaluating RHS so blocked prototype + // paths cannot trigger user callbacks or other RHS side effects. const pathKeys = this.getPathKeys(ref); let setPath = this.resolveSetPath(context, ref, pathKeys); @@ -151,6 +161,8 @@ export class SetValue extends Compile { return; } + // Resolve again after RHS evaluation to preserve legacy behavior where RHS + // side effects create a previously missing parent object. setPath = this.resolveSetPath(context, ref, pathKeys); if (setPath.blocked) {