diff --git a/src/full.js b/src/full.js index b5f86c3..bc9d95b 100644 --- a/src/full.js +++ b/src/full.js @@ -10,7 +10,13 @@ export function klona(x) { var i=0, k, list, tmp, str=Object.prototype.toString.call(x); - if (str === '[object Object]') { + // `str` trusts `Symbol.toStringTag`, which any object can set to spoof + // a recognized type and get misrouted below (crash), or set to an + // unrecognized value and fall through to `return x` (reference leak). + // `constructor` is unaffected by that spoofing, so also treat a plain + // object (or one with no prototype) as `[object Object]` regardless of + // what `str` claims. @see https://github.com/lukeed/klona/issues/54 + if (str === '[object Object]' || (x && (x.constructor === Object || x.constructor == null))) { tmp = Object.create(x.__proto__ || null); } else if (str === '[object Array]') { tmp = Array(x.length); diff --git a/src/index.js b/src/index.js index 4b866ba..37bab2b 100644 --- a/src/index.js +++ b/src/index.js @@ -3,7 +3,13 @@ export function klona(x) { var k, tmp, str=Object.prototype.toString.call(x); - if (str === '[object Object]') { + // `str` trusts `Symbol.toStringTag`, which any object can set to spoof + // a recognized type and get misrouted below (crash), or set to an + // unrecognized value and fall through to `return x` (reference leak). + // `constructor` is unaffected by that spoofing, so also treat a plain + // object (or one with no prototype) as `[object Object]` regardless of + // what `str` claims. @see https://github.com/lukeed/klona/issues/54 + if (str === '[object Object]' || (x && (x.constructor === Object || x.constructor == null))) { if (x.constructor !== Object && typeof x.constructor === 'function') { tmp = new x.constructor(); for (k in x) { diff --git a/src/json.js b/src/json.js index eae543d..9d110b2 100644 --- a/src/json.js +++ b/src/json.js @@ -7,7 +7,13 @@ export function klona(val) { return out; } - if (Object.prototype.toString.call(val) === '[object Object]') { + // `Object.prototype.toString` trusts `Symbol.toStringTag`, which any + // object can set to an unrecognized value and fall through to + // `return val` below (reference leak). `constructor` is unaffected by + // that spoofing, so also treat a plain object (or one with no + // prototype) as an object regardless of what the tag claims. + // @see https://github.com/lukeed/klona/issues/54 + if (Object.prototype.toString.call(val) === '[object Object]' || (val && (val.constructor === Object || val.constructor == null))) { out = {}; // null for (k in val) { if (k === '__proto__') { diff --git a/src/lite.js b/src/lite.js index bbd342e..94b004a 100644 --- a/src/lite.js +++ b/src/lite.js @@ -3,7 +3,13 @@ export function klona(x) { var k, tmp, str=Object.prototype.toString.call(x); - if (str === '[object Object]') { + // `str` trusts `Symbol.toStringTag`, which any object can set to an + // unrecognized value and fall through to `return x` below (reference + // leak). `constructor` is unaffected by that spoofing, so also treat a + // plain object (or one with no prototype) as `[object Object]` + // regardless of what `str` claims. + // @see https://github.com/lukeed/klona/issues/54 + if (str === '[object Object]' || (x && (x.constructor === Object || x.constructor == null))) { if (x.constructor !== Object && typeof x.constructor === 'function') { tmp = new x.constructor(); for (k in x) { diff --git a/test/full.js b/test/full.js index 816a9b7..3f665eb 100644 --- a/test/full.js +++ b/test/full.js @@ -13,6 +13,7 @@ suites.RegExps(klona); suites.Objects(klona); suites.Arrays(klona); +suites.ToStringTag(klona); suites.Functions(klona); suites.Pollutions(klona); diff --git a/test/index.js b/test/index.js index 46470d4..4a33445 100644 --- a/test/index.js +++ b/test/index.js @@ -13,6 +13,7 @@ suites.RegExps(klona); suites.Objects(klona); suites.Arrays(klona); +suites.ToStringTag(klona); suites.Functions(klona); suites.Pollutions(klona); diff --git a/test/json.js b/test/json.js index d32a942..57e2469 100644 --- a/test/json.js +++ b/test/json.js @@ -10,5 +10,6 @@ suites.Nully(klona); suites.Objects(klona); suites.Arrays(klona); +suites.ToStringTag(klona); suites.Pollutions(klona); diff --git a/test/lite.js b/test/lite.js index a406dc3..7d68557 100644 --- a/test/lite.js +++ b/test/lite.js @@ -13,6 +13,7 @@ suites.RegExps(klona); suites.Objects(klona); suites.Arrays(klona); +suites.ToStringTag(klona); suites.Functions(klona); suites.Pollutions(klona); diff --git a/test/suites/index.js b/test/suites/index.js index 7d5ccfc..800a093 100644 --- a/test/suites/index.js +++ b/test/suites/index.js @@ -11,6 +11,7 @@ export { default as Sets } from './set'; export { default as TypedArrays } from './typedarray'; export { default as Descriptors } from './descriptor'; export { default as Symbols } from './symbol'; +export { default as ToStringTag } from './tostringtag'; export { default as Dates } from './date'; export { default as RegExps } from './regexp'; diff --git a/test/suites/tostringtag.js b/test/suites/tostringtag.js new file mode 100644 index 0000000..da98aa4 --- /dev/null +++ b/test/suites/tostringtag.js @@ -0,0 +1,44 @@ +import { suite } from 'uvu'; +import * as assert from 'assert'; + +export default function (klona) { + const Spoofed = suite('Symbol.toStringTag spoofing'); + + // A plain object can set `Symbol.toStringTag` to a value that klona + // *does* special-case (eg, "Map"), which used to misroute it into that + // type's clone branch and crash on methods it doesn't have. + // @see https://github.com/lukeed/klona/issues/54 + Spoofed('recognized tag :: plain object is cloned, not misrouted', () => { + const input = { a: 1, b: { c: 2 }, [Symbol.toStringTag]: 'Map' }; + const output = klona(input); + + assert.notEqual(output, input, 'must return a new object, not the input'); + assert.equal(output.a, 1); + assert.deepEqual(output.b, { c: 2 }); + + output.a = 999; + output.b.c = 888; + assert.equal(input.a, 1, 'mutating output.a must not affect input'); + assert.equal(input.b.c, 2, 'mutating output.b must not affect input'); + }); + + // A plain object can set `Symbol.toStringTag` to a value klona does + // *not* special-case (eg, an arbitrary string), which used to fall + // through every branch and get returned by reference instead of cloned. + // @see https://github.com/lukeed/klona/issues/54 + Spoofed('unrecognized tag :: plain object is cloned, not returned by reference', () => { + const input = { a: 1, b: { c: 2 }, [Symbol.toStringTag]: 'Foobar' }; + const output = klona(input); + + assert.notEqual(output, input, 'must return a new object, not the input'); + assert.equal(output.a, 1); + assert.deepEqual(output.b, { c: 2 }); + + output.a = 999; + output.b.c = 888; + assert.equal(input.a, 1, 'mutating output.a must not affect input'); + assert.equal(input.b.c, 2, 'mutating output.b must not affect input'); + }); + + Spoofed.run(); +}