Skip to content

Commit

Permalink
Properly handle objects that inherit through __proto__ with no extra …
Browse files Browse the repository at this point in the history
…constructor by using more robust property ownership check and improve property safety checks
  • Loading branch information
kriszyp committed Feb 11, 2024
1 parent 50bcdb8 commit 288a906
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 15 deletions.
28 changes: 14 additions & 14 deletions pack.js
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ export class Packr extends Unpackr {
}
let constructor = value.constructor
if (constructor === Object) {
writeObject(value, true)
writeObject(value)
} else if (constructor === Array) {
packArray(value)
} else if (constructor === Map) {
Expand Down Expand Up @@ -511,8 +511,8 @@ export class Packr extends Unpackr {
if (type === 'function')
return pack(this.writeFunction && this.writeFunction(value));

// no extension found, write as object
writeObject(value, !value.hasOwnProperty) // if it doesn't have hasOwnProperty, don't do hasOwnProperty checks
// no extension found, write as plain object
writeObject(value)
}
}
}
Expand Down Expand Up @@ -599,13 +599,13 @@ export class Packr extends Unpackr {
}
}
} :
(object, safePrototype) => {
(object) => {
target[position++] = 0xde // always using map 16, so we can preallocate and set the length afterwards
let objectOffset = position - start
position += 2
let size = 0
for (let key in object) {
if (safePrototype || object.hasOwnProperty(key)) {
if (typeof object.hasOwnProperty !== 'function' || object.hasOwnProperty(key)) {
pack(key)
pack(object[key])
size++
Expand All @@ -617,12 +617,12 @@ export class Packr extends Unpackr {

const writeRecord = this.useRecords === false ? writePlainObject :
(options.progressiveRecords && !useTwoByteRecords) ? // this is about 2% faster for highly stable structures, since it only requires one for-in loop (but much more expensive when new structure needs to be written)
(object, safePrototype) => {
(object) => {
let nextTransition, transition = structures.transitions || (structures.transitions = Object.create(null))
let objectOffset = position++ - start
let wroteKeys
for (let key in object) {
if (safePrototype || object.hasOwnProperty(key)) {
if (typeof object.hasOwnProperty !== 'function' || object.hasOwnProperty(key)) {
nextTransition = transition[key]
if (nextTransition)
transition = nextTransition
Expand Down Expand Up @@ -661,10 +661,10 @@ export class Packr extends Unpackr {
insertNewRecord(transition, Object.keys(object), objectOffset, 0)
}
} :
(object, safePrototype) => {
(object) => {
let nextTransition, transition = structures.transitions || (structures.transitions = Object.create(null))
let newTransitions = 0
for (let key in object) if (safePrototype || object.hasOwnProperty(key)) {
for (let key in object) if (typeof object.hasOwnProperty !== 'function' || object.hasOwnProperty(key)) {
nextTransition = transition[key]
if (!nextTransition) {
nextTransition = transition[key] = Object.create(null)
Expand All @@ -684,16 +684,16 @@ export class Packr extends Unpackr {
}
// now write the values
for (let key in object)
if (safePrototype || object.hasOwnProperty(key)) {
if (typeof object.hasOwnProperty !== 'function' || object.hasOwnProperty(key)) {
pack(object[key])
}
}

// craete reference to useRecords if useRecords is a function
const checkUseRecords = typeof this.useRecords == 'function' && this.useRecords;

const writeObject = checkUseRecords ? (object, safePrototype) => {
checkUseRecords(object) ? writeRecord(object,safePrototype) : writePlainObject(object,safePrototype)
const writeObject = checkUseRecords ? (object) => {
checkUseRecords(object) ? writeRecord(object) : writePlainObject(object)
} : writeRecord

const makeRoom = (end) => {
Expand Down Expand Up @@ -798,7 +798,7 @@ export class Packr extends Unpackr {
target[insertionOffset + start] = keysTarget[0]
}
}
const writeStruct = (object, safePrototype) => {
const writeStruct = (object) => {
let newPosition = writeStructSlots(object, target, start, position, structures, makeRoom, (value, newPosition, notifySharedUpdate) => {
if (notifySharedUpdate)
return hasSharedUpdate = true;
Expand All @@ -812,7 +812,7 @@ export class Packr extends Unpackr {
return position;
}, this);
if (newPosition === 0) // bail and go to a msgpack object
return writeObject(object, true);
return writeObject(object);
position = newPosition;
}
}
Expand Down
7 changes: 7 additions & 0 deletions tests/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,13 @@ suite('msgpackr basic tests', function() {
assert.deepEqual(deserialized, data)
})

test('object with __proto__', function(){
const data = { foo: 'bar', __proto__: { isAdmin: true } };
var serialized = pack(data)
var deserialized = unpack(serialized)
assert.deepEqual(deserialized, { foo: 'bar' });
})

test('separate instances', function() {
const packr = new Packr({
structures: [['m', 'e'], ['action', 'share']]
Expand Down
4 changes: 3 additions & 1 deletion unpack.js
Original file line number Diff line number Diff line change
Expand Up @@ -967,8 +967,10 @@ function readKey() {
}

function asSafeString(property) {
// protect against expensive (DoS) string conversions
if (typeof property === 'string') return property;
if (typeof property === 'number') return property.toString();
if (typeof property === 'number' || typeof property === 'boolean' || typeof property === 'bigint') return property.toString();
if (property == null) return property + '';
throw new Error('Invalid property type for record', typeof property);
}
// the registration of the record definition extension (as "r")
Expand Down

0 comments on commit 288a906

Please sign in to comment.