vulnerable undefined property lookup that escalating prototype pollution to reflected XSS
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 2.9k
- Forks
- 461
- PR merge metrics
- No merged PRs in 30d
Description
Hi there!
I've identified several prototype pollution gadgets within the `dustjs` template engine that could potentially be leveraged by attackers to achieve client-side cross-site scripting (XSS) through prototype pollution vulnerability.
**In light of the findings, I kindly request your confirmation of this potential issue 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 `dustjs` as the backend template engine, and there is another prototype pollution vulnerability in the application, then the attacker could leverage the found gadgets inside the template engine to escalate the prototype pollution to reflected XSS that affects all the client users.
## Proof of Concept
Below, I present a Proof of Concept (PoC) to demonstrate the identified gadgets within the `dustjs@3.0.1` template engine. This particular gadget activates when there's a non-iterative variable lookup (`rootdir`) within an array context in the template (`{#names}...{/names}`). The pertinent code segment can be found in the `dustjs` run-time function [`_get`](https://github.com/linkedin/dustjs/blob/master/dist/dust-full.js#L398).
```
// prototype pollution to XSS
Object.prototype.rootdir = "; onerror=alert(1);//"
var tmpl = dust.compile("{#names}{~n}{/names}", "templateName");
dust.loadSource(tmpl);
dust.render("templateName", { rootdir: "/www", names: [ { name: "Moe" }, { name: "Larry" }, { name: "Curly" } ] }, function(err, out) {
if(err) console.error(err);
else console.log(out);
});
```
Output:
```
```
## 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.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the _get runtime function in dist/dust-full.js around line 398 and reproduce the supplied dustjs@3.0.1 PoC. Trace the non-iterative lookup inside an array context, then inspect related undefined-property lookups. Done means inherited properties such as rootdir are no longer usable as template values, with the PoC demonstrating safe output.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100