Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/CodeGenerator.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ CodeGenerator.prototype.compile = function (namespace) {
' $$processScope(scope)',
' ' + code,
' },',
" code: '" + code + "'",
' code: ' + JSON.stringify(code),
'}'
].join('\n')

Expand Down
5 changes: 5 additions & 0 deletions lib/Interpreter.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ const Interpreter = function (owner, options) {
rawCallExpressionElements: false,
applyFactoryToScope: false
}, options)

const factory = this.options.factory
if (typeof factory !== 'string' || !/^[a-zA-Z_$][a-zA-Z0-9_$]*(\.[a-zA-Z_$][a-zA-Z0-9_$]*)*$/.test(factory)) {
throw new Error('factory must be a valid JS property access path (e.g. "ns.factory")')
}
}

extend(Interpreter.prototype, types)
Expand Down
2 changes: 1 addition & 1 deletion lib/node/AssignmentNode.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict'

module.exports = function (node) {
return 'scope["' + node.name + '"] = ' + this.next(node.expr)
return 'scope[' + JSON.stringify(node.name) + '] = ' + this.next(node.expr)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-critical critical

While this change prevents code injection, it is still vulnerable to prototype pollution. An assignment where node.name is __proto__ can modify the prototype of the scope object. This can lead to security vulnerabilities like remote code execution. It's crucial to forbid assignments to __proto__.

Suggested change
return 'scope[' + JSON.stringify(node.name) + '] = ' + this.next(node.expr)
return node.name === '__proto__' ? 'throw new Error("Assignment to __proto__ is not allowed.")' : 'scope[' + JSON.stringify(node.name) + '] = ' + this.next(node.expr);

}
3 changes: 2 additions & 1 deletion lib/node/ConstantNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ module.exports = function (node) {
if (this.options.raw) {
return node.value
}
return this.options.factory + '(' + node.value + ')'
const value = node.valueType === 'string' ? JSON.stringify(node.value) : node.value
return this.options.factory + '(' + value + ')'
}
2 changes: 1 addition & 1 deletion lib/node/FunctionNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
const SymbolNode = require('mr-parser').nodeTypes.SymbolNode

const functionProxy = function (node) {
return '$$mathCodegen.functionProxy(' + this.next(new SymbolNode(node.name)) + ', "' + node.name + '")'
return '$$mathCodegen.functionProxy(' + this.next(new SymbolNode(node.name)) + ', ' + JSON.stringify(node.name) + ')'
}

module.exports = function (node) {
Expand Down
2 changes: 1 addition & 1 deletion lib/node/SymbolNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

module.exports = function (node) {
const id = node.name
return '$$mathCodegen.getProperty("' + id + '", scope, ns)'
return '$$mathCodegen.getProperty(' + JSON.stringify(id) + ', scope, ns)'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

Accessing __proto__ can leak Object.prototype. This information exposure might not be intended and could provide valuable information to an attacker for chaining vulnerabilities. It's safer to explicitly disallow access to __proto__.

Suggested change
return '$$mathCodegen.getProperty(' + JSON.stringify(id) + ', scope, ns)'
return id === '__proto__' ? 'throw new Error("Access to __proto__ is not allowed.")' : '$$mathCodegen.getProperty(' + JSON.stringify(id) + ', scope, ns)';

}
18 changes: 17 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading