Skip to content

Commit 68d8df5

Browse files
jaylinskijaylinski
committed
Fix security issues
Fixes GHSA-2w6w-674q-4c4q, GHSA-xhpv-hc6g-r9c6, GHSA-3mfm-83xf-c92r, GHSA-2qvq-rjwj-gvw9, GHSA-9cx6-37pm-9jff, GHSA-7rx3-28cr-v5wh, GHSA-442j-39wm-28r2, GHSA-xjpj-3mr7-gcpf
1 parent b2a0831 commit 68d8df5

20 files changed

+541
-38
lines changed

‎lib/handlebars/compiler/base.js‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import parser from './parser';
22
import WhitespaceControl from './whitespace-control';
33
import * as Helpers from './helpers';
4+
import Exception from '../exception';
45
import { extend } from '../utils';
56

67
export { parser };
@@ -11,6 +12,9 @@ extend(yy, Helpers);
1112
export function parseWithoutProcessing(input, options) {
1213
// Just return if an already-compiled AST was passed in.
1314
if (input.type === 'Program') {
15+
// When a pre-parsed AST is passed in, validate all node values to prevent
16+
// code injection via type-confused literals.
17+
validateInputAst(input);
1418
return input;
1519
}
1620

@@ -32,3 +36,66 @@ export function parse(input, options) {
3236

3337
return strip.accept(ast);
3438
}
39+
40+
function validateInputAst(ast) {
41+
validateAstNode(ast);
42+
}
43+
44+
function validateAstNode(node) {
45+
if (node == null) {
46+
return;
47+
}
48+
49+
if (Array.isArray(node)) {
50+
node.forEach(validateAstNode);
51+
return;
52+
}
53+
54+
if (typeof node !== 'object') {
55+
return;
56+
}
57+
58+
if (node.type === 'PathExpression') {
59+
if (!isValidDepth(node.depth)) {
60+
throw new Exception(
61+
'Invalid AST: PathExpression.depth must be an integer'
62+
);
63+
}
64+
if (!Array.isArray(node.parts)) {
65+
throw new Exception('Invalid AST: PathExpression.parts must be an array');
66+
}
67+
for (let i = 0; i < node.parts.length; i++) {
68+
if (typeof node.parts[i] !== 'string') {
69+
throw new Exception(
70+
'Invalid AST: PathExpression.parts must only contain strings'
71+
);
72+
}
73+
}
74+
} else if (node.type === 'NumberLiteral') {
75+
if (typeof node.value !== 'number' || !isFinite(node.value)) {
76+
throw new Exception('Invalid AST: NumberLiteral.value must be a number');
77+
}
78+
} else if (node.type === 'BooleanLiteral') {
79+
if (typeof node.value !== 'boolean') {
80+
throw new Exception(
81+
'Invalid AST: BooleanLiteral.value must be a boolean'
82+
);
83+
}
84+
}
85+
86+
Object.keys(node).forEach(propertyName => {
87+
if (propertyName === 'loc') {
88+
return;
89+
}
90+
validateAstNode(node[propertyName]);
91+
});
92+
}
93+
94+
function isValidDepth(depth) {
95+
return (
96+
typeof depth === 'number' &&
97+
isFinite(depth) &&
98+
Math.floor(depth) === depth &&
99+
depth >= 0
100+
);
101+
}

‎lib/handlebars/compiler/javascript-compiler.js‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,9 +686,18 @@ JavaScriptCompiler.prototype = {
686686
let foundDecorator = this.nameLookup('decorators', name, 'decorator'),
687687
options = this.setupHelperArgs(name, paramSize);
688688

689+
// Store the resolved decorator in a variable and verify it is a function before
690+
// calling it. Without this, unregistered decorators can cause an unhandled TypeError
691+
// (calling undefined), which crashes the process — enabling Denial of Service.
692+
this.decorators.push(['var decorator = ', foundDecorator, ';']);
693+
this.decorators.push([
694+
'if (typeof decorator !== "function") { throw new Error(',
695+
this.quotedString('Missing decorator: "' + name + '"'),
696+
'); }'
697+
]);
689698
this.decorators.push([
690699
'fn = ',
691-
this.decorators.functionCall(foundDecorator, '', [
700+
this.decorators.functionCall('decorator', '', [
692701
'fn',
693702
'props',
694703
'container',

‎lib/handlebars/internal/proto-access.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export function createProtoAccessControl(runtimeOptions) {
1616
methodWhiteList['__defineGetter__'] = false;
1717
methodWhiteList['__defineSetter__'] = false;
1818
methodWhiteList['__lookupGetter__'] = false;
19+
methodWhiteList['__lookupSetter__'] = false;
1920
extend(methodWhiteList, runtimeOptions.allowedProtoMethods);
2021

2122
return {

‎lib/handlebars/runtime.js‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ export function template(templateSpec, env) {
138138
for (let i = 0; i < len; i++) {
139139
let result = depths[i] && container.lookupProperty(depths[i], name);
140140
if (result != null) {
141-
return depths[i][name];
141+
return result;
142142
}
143143
}
144144
},
@@ -349,21 +349,21 @@ export function wrapProgram(
349349
export function resolvePartial(partial, context, options) {
350350
if (!partial) {
351351
if (options.name === '@partial-block') {
352-
partial = options.data['partial-block'];
352+
partial = lookupOwnProperty(options.data, 'partial-block');
353353
} else {
354-
partial = options.partials[options.name];
354+
partial = lookupOwnProperty(options.partials, options.name);
355355
}
356356
} else if (!partial.call && !options.name) {
357357
// This is a dynamic partial that returned a string
358358
options.name = partial;
359-
partial = options.partials[partial];
359+
partial = lookupOwnProperty(options.partials, partial);
360360
}
361361
return partial;
362362
}
363363

364364
export function invokePartial(partial, context, options) {
365365
// Use the current closure context to save the partial-block if this partial
366-
const currentPartialBlock = options.data && options.data['partial-block'];
366+
const currentPartialBlock = lookupOwnProperty(options.data, 'partial-block');
367367
options.partial = true;
368368
if (options.ids) {
369369
options.data.contextPath = options.ids[0] || options.data.contextPath;
@@ -404,6 +404,12 @@ export function noop() {
404404
return '';
405405
}
406406

407+
function lookupOwnProperty(obj, name) {
408+
if (obj && Object.prototype.hasOwnProperty.call(obj, name)) {
409+
return obj[name];
410+
}
411+
}
412+
407413
function initData(context, data) {
408414
if (!data || !('root' in data)) {
409415
data = data ? createFrame(data) : {};

‎lib/precompiler.js‎

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -196,16 +196,24 @@ module.exports.cli = function(opts) {
196196

197197
const objectName = opts.partial ? 'Handlebars.partials' : 'templates';
198198

199+
if (opts.namespace && !isValidNamespace(opts.namespace)) {
200+
throw new Handlebars.Exception('Invalid namespace format');
201+
}
202+
199203
let output = new SourceNode();
200204
if (!opts.simple) {
201205
if (opts.amd) {
206+
const runtimeModulePath =
207+
(opts.handlebarPath || '') + 'handlebars.runtime';
202208
output.add(
203-
"define(['" +
204-
opts.handlebarPath +
205-
'handlebars.runtime\'], function(Handlebars) {\n Handlebars = Handlebars["default"];'
209+
'define([' +
210+
quoteForJavaScript(runtimeModulePath) +
211+
'], function(Handlebars) {\n Handlebars = Handlebars["default"];'
206212
);
207213
} else if (opts.commonjs) {
208-
output.add('var Handlebars = require("' + opts.commonjs + '");');
214+
output.add(
215+
'var Handlebars = require(' + quoteForJavaScript(opts.commonjs) + ');'
216+
);
209217
} else {
210218
output.add('(function() {\n');
211219
}
@@ -255,9 +263,9 @@ module.exports.cli = function(opts) {
255263
}
256264
output.add([
257265
objectName,
258-
"['",
259-
template.name,
260-
"'] = template(",
266+
'[',
267+
quoteForJavaScript(template.name),
268+
'] = template(',
261269
precompiled,
262270
');\n'
263271
]);
@@ -277,7 +285,9 @@ module.exports.cli = function(opts) {
277285
}
278286

279287
if (opts.map) {
280-
output.add('\n//# sourceMappingURL=' + opts.map + '\n');
288+
output.add(
289+
'\n//# sourceMappingURL=' + sanitizeSourceMapComment(opts.map) + '\n'
290+
);
281291
}
282292

283293
output = output.toStringWithSourceMap();
@@ -307,6 +317,33 @@ function arrayCast(value) {
307317
return value;
308318
}
309319

320+
/*
321+
* Safely quotes a value for embedding in generated JavaScript strings
322+
*
323+
* Uses JSON.stringify which handles all special characters.
324+
*/
325+
function quoteForJavaScript(value) {
326+
return JSON.stringify(String(value));
327+
}
328+
329+
/**
330+
* Validates that a namespace is a legitimate dotted JavaScript identifier
331+
* (e.g. "App.templates") to prevent arbitrary code injection
332+
*/
333+
function isValidNamespace(namespace) {
334+
return /^[A-Za-z_$][A-Za-z0-9_$]*(\.[A-Za-z_$][A-Za-z0-9_$]*)*$/.test(
335+
namespace
336+
);
337+
}
338+
339+
/**
340+
* Strips line terminators from source map URLs to prevent injection of new
341+
* JavaScript lines via the sourceMappingURL comment
342+
*/
343+
function sanitizeSourceMapComment(value) {
344+
return String(value).replace(/[\r\n\u2028\u2029]/g, '');
345+
}
346+
310347
/**
311348
* Run uglify to minify the compiled template, if uglify exists in the dependencies.
312349
*

‎spec/compiler.js‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,146 @@ describe('compiler', function() {
128128
);
129129
});
130130

131+
it('should reject AST with invalid PathExpression depth', function() {
132+
shouldThrow(
133+
function() {
134+
Handlebars.compile({
135+
type: 'Program',
136+
body: [
137+
{
138+
type: 'MustacheStatement',
139+
escaped: true,
140+
strip: { open: false, close: false },
141+
path: {
142+
type: 'PathExpression',
143+
data: false,
144+
depth: '0',
145+
parts: ['this'],
146+
original: 'this'
147+
},
148+
params: []
149+
}
150+
]
151+
})();
152+
},
153+
Error,
154+
'Invalid AST: PathExpression.depth must be an integer'
155+
);
156+
});
157+
158+
it('should reject AST with non-array PathExpression parts', function() {
159+
shouldThrow(
160+
function() {
161+
Handlebars.compile({
162+
type: 'Program',
163+
body: [
164+
{
165+
type: 'MustacheStatement',
166+
escaped: true,
167+
strip: { open: false, close: false },
168+
path: {
169+
type: 'PathExpression',
170+
data: false,
171+
depth: 0,
172+
parts: 'this',
173+
original: 'this'
174+
},
175+
params: []
176+
}
177+
]
178+
})();
179+
},
180+
Error,
181+
'Invalid AST: PathExpression.parts must be an array'
182+
);
183+
});
184+
185+
it('should reject AST with non-string PathExpression part', function() {
186+
shouldThrow(
187+
function() {
188+
Handlebars.compile({
189+
type: 'Program',
190+
body: [
191+
{
192+
type: 'MustacheStatement',
193+
escaped: true,
194+
strip: { open: false, close: false },
195+
path: {
196+
type: 'PathExpression',
197+
data: false,
198+
depth: 0,
199+
parts: [1],
200+
original: 'this'
201+
},
202+
params: []
203+
}
204+
]
205+
})();
206+
},
207+
Error,
208+
'Invalid AST: PathExpression.parts must only contain strings'
209+
);
210+
});
211+
212+
it('should reject AST with invalid BooleanLiteral value type', function() {
213+
shouldThrow(
214+
function() {
215+
Handlebars.compile({
216+
type: 'Program',
217+
body: [
218+
{
219+
type: 'MustacheStatement',
220+
escaped: true,
221+
strip: { open: false, close: false },
222+
path: {
223+
type: 'PathExpression',
224+
data: false,
225+
depth: 0,
226+
parts: ['if'],
227+
original: 'if'
228+
},
229+
params: [
230+
{
231+
type: 'BooleanLiteral',
232+
value: 'true',
233+
original: true
234+
}
235+
]
236+
}
237+
]
238+
})();
239+
},
240+
Error,
241+
'Invalid AST: BooleanLiteral.value must be a boolean'
242+
);
243+
});
244+
245+
it('should ignore loc metadata while validating AST nodes', function() {
246+
equal(
247+
Handlebars.compile({
248+
type: 'Program',
249+
meta: null,
250+
loc: { source: 'fake', start: { line: 1, column: 0 } },
251+
body: [{ type: 'ContentStatement', value: 'Hello' }]
252+
})(),
253+
'Hello'
254+
);
255+
});
256+
257+
it('should accept AST with valid NumberLiteral values', function() {
258+
equal(
259+
Handlebars.compile(Handlebars.parse('{{lookup this 1}}'))(['a', 'b']),
260+
'b'
261+
);
262+
});
263+
264+
it('should accept AST with valid BooleanLiteral values', function() {
265+
equal(
266+
Handlebars.compile(Handlebars.parse('{{#if true}}ok{{/if}}'))({}),
267+
'ok'
268+
);
269+
});
270+
131271
it('can pass through an empty string', function() {
132272
equal(Handlebars.compile('')(), '');
133273
});

0 commit comments

Comments
 (0)
⚡