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

Fix security bug about prototype pollution #1330

Merged
merged 1 commit into from
Nov 25, 2020

Conversation

ChenKS12138
Copy link
Contributor

@ChenKS12138 ChenKS12138 commented Nov 24, 2020

Summary

Proposed change:

Add check in function Frame.prototype.lookup to ensure name is own property of this.variables.

This is a security bug. The current version of nunjucks can be attacked by prototype pollution.
What I expected isthis is payload2 content is function(){ return global.process.mainModule.require('child_process').execSync('ls') }() , but the function returns this is payload2 content is main.js node_modules package.json yarn.lock.

Closes #1331.

The sample code is as follows.

const nunjucks = require("nunjucks");

nunjucks.configure({
  autoescape: true,
});

const template = nunjucks.compile(" content is {{ content }} ");

const payload = { };

payload.__proto__.content =
  " function(){ return global.process.mainModule.require('child_process').execSync('whoami') }() ";

console.log("this is payload2 ", template.render(payload));

image

Checklist

I've completed the checklist below to ensure I didn't forget anything. This makes reviewing this PR as easy as possible for the maintainers. And it gets this change released as soon as possible.

  • Proposed change helps towards purpose of this project.
  • Documentation is added / updated to describe proposed change. No documentation to update; this is a bug fix only
  • Tests are added / updated to cover proposed change.
  • Changelog has an entry for proposed change (if user-facing fix or feature).

@fdintino fdintino merged commit aa9e5b9 into mozilla:master Nov 25, 2020
@fdintino
Copy link
Collaborator

Thanks! I'll put out a new release tomorrow morning (EST time)

@ChenKS12138
Copy link
Contributor Author

My pleasure!

@DevRCRun
Copy link

Thanks! I'll put out a new release tomorrow morning (EST time)

Apologies for the noise, however I've only just come across this while browsing for something else but does a new release still need to be made? Or would the path to exploit this be non trivial?

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

Successfully merging this pull request may close these issues.

Security bug about prototype pollution
3 participants