Repository: mozilla/nunjucks (js)

Issue:
Security bug about prototype pollution
This is a security bug. The current version of nunjucks can be attacked by prototype pollution.
What I expected is`this 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 #1330 .

Environment
> Mac os 10.15.7
 Nodejs 12.18.1
nunjucks 3.2.2

The sample code is as follows.
```javascript
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](https://user-images.githubusercontent.com/42082890/100094205-9cc29480-2e93-11eb-8365-6b2d191de84e.png)



Pull request:
Fix security bug about prototype pollution
## 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 is`this 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.
```javascript
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](https://user-images.githubusercontent.com/42082890/100094205-9cc29480-2e93-11eb-8365-6b2d191de84e.png)



## 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.

* [x] Proposed change helps towards [*purpose of this project*](https://github.com/mozilla/nunjucks/blob/master/CONTRIBUTING.md#purpose).
* [x] [*Documentation*](https://github.com/mozilla/nunjucks/tree/master/docs/) is added / updated to describe proposed change.  **No documentation to update; this is a bug fix only**
* [x] [*Tests*](https://github.com/mozilla/nunjucks/tree/master/tests) are added / updated to cover proposed change.
* [x] [*Changelog*](https://github.com/mozilla/nunjucks/blob/master/CHANGELOG.md) has an entry for proposed change (if user-facing fix or feature).

<!-- Tick of items by replacing `[ ]` by `[x]` -->

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ad9fed8..bae08db 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,10 @@ Unreleased
 * Add `base` arg to
   [`int` filter](https://mozilla.github.io/nunjucks/templating.html#int).
 * Move `chokidar` to `peerDependencies` and mark it `optional` in `peerDependenciesMeta`.
+* Fix prototype pollution issue for template variables. Merge of
+  [#1330](https://github.com/mozilla/nunjucks/pull/1330); fixes
+  [#1331](https://github.com/mozilla/nunjucks/issues/1331). Thanks
+  [ChenKS12138](https://github.com/ChenKS12138)!
 
 3.2.2 (Jul 20 2020)
 -------------------
diff --git a/nunjucks/src/runtime.js b/nunjucks/src/runtime.js
index d57c741..4d1cb4b 100644
--- a/nunjucks/src/runtime.js
+++ b/nunjucks/src/runtime.js
@@ -12,7 +12,7 @@ var supportsIterators = (
 // variables, for example.
 class Frame {
   constructor(parent, isolateWrites) {
-    this.variables = {};
+    this.variables = Object.create(null);
     this.parent = parent;
     this.topLevel = false;
     // if this is true, writes (set) should never propagate upwards past


Test patch:
diff --git a/tests/runtime.js b/tests/runtime.js
index 4b104f6..1a799ba 100644
--- a/tests/runtime.js
+++ b/tests/runtime.js
@@ -110,5 +110,21 @@
 
       finish(done);
     });
+
+    it('should not read variables property from Object.prototype', function(done) {
+      var payload = 'function(){ return 1+2; }()';
+      var data = {};
+      Object.getPrototypeOf(data).payload = payload;
+
+      render('{{ payload }}', data, {
+        noThrow: true
+      }, function(err, res) {
+        expect(err).to.equal(null);
+        expect(res).to.equal(payload);
+      });
+      delete Object.getPrototypeOf(data).payload;
+
+      finish(done);
+    });
   });
 }());