Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/decode.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ function tokenToMap (token, tokeniser, options) {
if (useMaps) {
// @ts-ignore TODO reconsider this .. maybe needs to be strict about key types
m.set(key, value)
} else if (key === '__proto__') {
// a plain assignment would invoke the prototype setter instead of creating
// an own property, losing the entry and mutating the object's prototype
Object.defineProperty(obj, key, { value, configurable: true, enumerable: true, writable: true })
} else {
// @ts-ignore TODO reconsider this .. maybe needs to be strict about key types
obj[key] = value
Expand Down
14 changes: 14 additions & 0 deletions test/test-decode-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,18 @@ describe('decode errors', () => {
assert.throws(() => decode(fromHex('a3636261720363666f6f0163666f6f02'), { rejectDuplicateMapKeys: true }), /CBOR decode error: found repeat map key "foo"/)
assert.throws(() => decode(fromHex('a3636261720363666f6f0163666f6f02'), { useMaps: true, rejectDuplicateMapKeys: true }), /CBOR decode error: found repeat map key "foo"/)
})

it('__proto__ map key decodes as an own property without polluting the prototype', () => {
// {"__proto__": 1}
const obj = decode(fromHex('a1695f5f70726f746f5f5f01'))
assert.ok(Object.hasOwn(obj, '__proto__'), 'has own __proto__ property')
assert.strictEqual(obj.__proto__, 1) // eslint-disable-line no-proto
assert.strictEqual(Object.getPrototypeOf(obj), Object.prototype, 'prototype is unchanged')
Comment on lines +70 to +71

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should run this without your change in decode.js, I'm pretty sure it'll pass anyway because __proto__ can only be set to an object or null, not primitives/scalars like 1.
Change the test to do something like __proto__ = {naughty:'obj'} (cborg cli tells me it would be a1695f5f70726f746f5f5fa1676e617567687479636f626a) and then I think you have a proper test.

Other than this, it's a good change @spokodev thanks!

assert.deepStrictEqual(Object.keys(obj), ['__proto__'])
})

it('rejectDuplicateMapKeys enabled on duplicate __proto__ keys', () => {
// {"__proto__": 1, "__proto__": 2}
assert.throws(() => decode(fromHex('a2695f5f70726f746f5f5f01695f5f70726f746f5f5f02'), { rejectDuplicateMapKeys: true }), /CBOR decode error: found repeat map key "__proto__"/)
})
})
Loading