Fix object proto pollution #113
Conversation
src/inserters.js
Outdated
| desc.configurable = true; | ||
| if (prop === 'value') { | ||
| desc.writable = true; | ||
| if (Object.hasOwnProperty.call(map, proto)) { |
There was a problem hiding this comment.
can we instead do:
const protos = Object.getOwnPropertyNames(map);
for (let i = 0; i < protos.length; i++) {
const proto = protos[i];
const funcs = map[proto];
...?
That way we save the extra indentation, and this should be safe because Object is a native and therefore protos too
There was a problem hiding this comment.
(protos could also be declared in the upper scope, outside of the function, your call)
There was a problem hiding this comment.
Good ideas! the 2nd will save the extra call. on it
test/inserters.js
Outdated
| @@ -184,4 +184,22 @@ describe('test DOM insertions', async function () { | |||
| }); | |||
| expect(result).toBe('V'); | |||
| }); | |||
|
|
|||
| it('should fail to use atob of an iframe added after Object.prototype pollution', async function () { | |||
There was a problem hiding this comment.
excellent test, I think this would be more appropriate at /test/overwrites.js. Also can you add the // reference: https://github.com/LavaMoat/snow/pull/112 comment please?
There was a problem hiding this comment.
Thank you @mmndaniel!
A few minor requests if you don't mind. Also, please add and commit snow.js and snow.prod.js files after running yarn build locally (you can see in former PRs we add the dst files with each change. we do this because we'd like each version to be accessible via servers such as unpkg out of the box)
There was a problem hiding this comment.
Awesome work, super important!
Thanks again for the help @mmndaniel 🙏
(first external PR ever btw 🎉)
An attempt to fix #112