Skip to content

Commit

Permalink
🐛 fix getStaticValue security issue
Browse files Browse the repository at this point in the history
  • Loading branch information
mysticatea committed Aug 20, 2019
1 parent 587cca2 commit 08158db
Show file tree
Hide file tree
Showing 2 changed files with 131 additions and 15 deletions.
115 changes: 102 additions & 13 deletions src/get-static-value.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* globals BigInt */

import { findVariable } from "./find-variable"

const builtinNames = Object.freeze(
Expand All @@ -14,9 +16,7 @@ const builtinNames = Object.freeze(
"decodeURIComponent",
"encodeURI",
"encodeURIComponent",
"Error",
"escape",
"EvalError",
"Float32Array",
"Float64Array",
"Function",
Expand All @@ -37,26 +37,97 @@ const builtinNames = Object.freeze(
"parseInt",
"Promise",
"Proxy",
"RangeError",
"ReferenceError",
"Reflect",
"RegExp",
"Set",
"String",
"Symbol",
"SyntaxError",
"TypeError",
"Uint16Array",
"Uint32Array",
"Uint8Array",
"Uint8ClampedArray",
"undefined",
"unescape",
"URIError",
"WeakMap",
"WeakSet",
])
)
const callAllowed = new Set(
[
Array.isArray,
typeof BigInt === "function" ? BigInt : undefined,
Boolean,
Date,
Date.parse,
decodeURI,
decodeURIComponent,
encodeURI,
encodeURIComponent,
escape,
isFinite,
isNaN,
isPrototypeOf,
...Object.getOwnPropertyNames(Math)
.map(k => Math[k])
.filter(f => typeof f === "function"),
Number,
Number.isFinite,
Number.isNaN,
Number.parseFloat,
Number.parseInt,
Object,
Object.entries, //eslint-disable-line @mysticatea/node/no-unsupported-features/es-builtins
Object.is,
Object.isExtensible,
Object.isFrozen,
Object.isSealed,
Object.keys,
Object.values, //eslint-disable-line @mysticatea/node/no-unsupported-features/es-builtins
parseFloat,
parseInt,
RegExp,
String,
String.fromCharCode,
String.fromCodePoint,
String.raw,
Symbol,
Symbol.for,
Symbol.keyFor,
unescape,
].filter(f => typeof f === "function")
)
const callPassThrough = new Set([
Object.freeze,
Object.preventExtensions,
Object.seal,
])

/**
* Get the property descriptor.
* @param {object} object The object to get.
* @param {string|number|symbol} name The property name to get.
*/
function getPropertyDescriptor(object, name) {
let x = object
while ((typeof x === "object" || typeof x === "function") && x !== null) {
const d = Object.getOwnPropertyDescriptor(x, name)
if (d) {
return d
}
x = Object.getPrototypeOf(x)
}
return null
}

/**
* Check if a property is getter or not.
* @param {object} object The object to check.
* @param {string|number|symbol} name The property name to check.
*/
function isGetter(object, name) {
const d = getPropertyDescriptor(object, name)
return d != null && d.get != null
}

/**
* Get the element values of a given node list.
Expand Down Expand Up @@ -176,13 +247,23 @@ const operations = Object.freeze({
if (object != null && property != null) {
const receiver = object.value
const methodName = property.value
return { value: receiver[methodName](...args) }
if (callAllowed.has(receiver[methodName])) {
return { value: receiver[methodName](...args) }
}
if (callPassThrough.has(receiver[methodName])) {
return { value: args[0] }
}
}
} else {
const callee = getStaticValueR(calleeNode, initialScope)
if (callee != null) {
const func = callee.value
return { value: func(...args) }
if (callAllowed.has(func)) {
return { value: func(...args) }
}
if (callPassThrough.has(func)) {
return { value: args[0] }
}
}
}
}
Expand Down Expand Up @@ -240,7 +321,7 @@ const operations = Object.freeze({
// It was a RegExp/BigInt literal, but Node.js didn't support it.
return null
}
return node
return { value: node.value }
},

LogicalExpression(node, initialScope) {
Expand Down Expand Up @@ -268,7 +349,11 @@ const operations = Object.freeze({
? getStaticValueR(node.property, initialScope)
: { value: node.property.name }

if (object != null && property != null) {
if (
object != null &&
property != null &&
!isGetter(object.value, property.value)
) {
return { value: object.value[property.value] }
}
return null
Expand All @@ -280,7 +365,9 @@ const operations = Object.freeze({

if (callee != null && args != null) {
const Func = callee.value
return { value: new Func(...args) }
if (callAllowed.has(Func)) {
return { value: new Func(...args) }
}
}

return null
Expand Down Expand Up @@ -339,7 +426,9 @@ const operations = Object.freeze({
const strings = node.quasi.quasis.map(q => q.value.cooked)
strings.raw = node.quasi.quasis.map(q => q.value.raw)

return { value: func(strings, ...expressions) }
if (func === String.raw) {
return { value: func(strings, ...expressions) }
}
}

return null
Expand Down
31 changes: 29 additions & 2 deletions test/get-static-value.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ describe("The 'getStaticValue' function", () => {
{ code: "Symbol[iterator]", expected: null },
{ code: "Object.freeze", expected: { value: Object.freeze } },
{ code: "Object.xxx", expected: { value: undefined } },
{ code: "new Array(2)", expected: { value: new Array(2) } },
{ code: "new Array(2)", expected: null },
{ code: "new Array(len)", expected: null },
{ code: "({})", expected: { value: {} } },
{
Expand Down Expand Up @@ -116,6 +116,33 @@ const aMap = Object.freeze({
;\`on\${eventName} : \${aMap[eventName]}\``,
expected: { value: "onclick : 777" },
},
{
code: 'Function("return process.env.npm_name")()',
expected: null,
},
{
code: 'new Function("return process.env.npm_name")()',
expected: null,
},
{
code:
'({}.constructor.constructor("return process.env.npm_name")())',
expected: null,
},
{
code:
'JSON.stringify({a:1}, new {}.constructor.constructor("console.log(\\"code injected\\"); process.exit(1)"), 2)',
expected: null,
},
{
code:
'Object.create(null, {a:{get:new {}.constructor.constructor("console.log(\\"code injected\\"); process.exit(1)")}}).a',
expected: null,
},
{
code: "RegExp.$1",
expected: null,
},
]) {
it(`should return ${JSON.stringify(expected)} from ${code}`, () => {
const linter = new eslint.Linter()
Expand All @@ -138,7 +165,7 @@ const aMap = Object.freeze({
if (actual == null) {
assert.strictEqual(actual, expected)
} else {
assert.deepStrictEqual(actual.value, expected.value)
assert.deepStrictEqual(actual, expected)
}
})
}
Expand Down

0 comments on commit 08158db

Please sign in to comment.