Skip to content

Harden against prototype pollution.#275

Open
sayrer wants to merge 5 commits into
masterfrom
harden_prototype_pollution
Open

Harden against prototype pollution.#275
sayrer wants to merge 5 commits into
masterfrom
harden_prototype_pollution

Conversation

@sayrer

@sayrer sayrer commented Dec 18, 2021

Copy link
Copy Markdown
Contributor

This makes it more difficult for RCEs in other libraries to overwrite the prototype chain to manipulate Hogan, but also prevents authors from manipulating the prototype chain on purpose. I think the latter is rare enough that this change is worth it.

This first patch just fixes node.indent. See #274.

@sayrer sayrer changed the title Harden node.indent against prototype pollution. Harden createPartials against prototype pollution. Dec 18, 2021
@sayrer sayrer changed the title Harden createPartials against prototype pollution. Harden against prototype pollution. Dec 18, 2021
@sayrer

sayrer commented Dec 18, 2021

Copy link
Copy Markdown
Contributor Author

Well, I typoed this, but it does fix the pollution. Will fix.

@ex1st

ex1st commented Dec 19, 2021

Copy link
Copy Markdown

Hi @sayrer, What do you think about that solution?

var context = Object.create(null, { 
  code: {
    value: "",
    writable: true,
    enumerable: true,
    configurable: true
  }, 
  subs: {
    value: Object.create(null),
    writable: true,
    enumerable: true,
    configurable: true
  }, 
  partials: {
    value: Object.create(null),
    writable: true,
    enumerable: true,
    configurable: true
  }
});

@sayrer

sayrer commented Dec 19, 2021

Copy link
Copy Markdown
Contributor Author

Hi @sayrer, What do you think about that solution?

It doesn't quite work, but that is another way.

Welcome to Node.js v17.2.0.
Type ".help" for more information.
> var context = Object.create(null, { 
...   code: {
.....     value: "",
.....     writable: true,
.....     enumerable: true,
.....     configurable: true
.....   }}); 
undefined
> context
[Object: null prototype] { code: '' }
> context.__proto__
undefined
> context.code.__proto__
{}
> context.code.__proto__.indent = "foo"
'foo'
> context.code.__proto__.indent
'foo'

@sayrer

sayrer commented Dec 19, 2021

Copy link
Copy Markdown
Contributor Author

It looks like I'll have to fix the test harness before I check this in, and your way will still break anyone relying on the prototype chain (probably unintentionally). But I'll see which one is cleaner in the end.

@ex1st

ex1st commented Dec 20, 2021

Copy link
Copy Markdown

Also, another simple way - predefined empty prefix

var context = { code: '', subs: {}, partials: {}, prefix: '' };

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.

2 participants