diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e0eb43..edc57fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +### Upcoming + +- Unserialize bug/security fixes: (by @souvlakias) + - Ensures object keys are either `number` or `string`, and not equal to `__proto__`. + - Ensures serializable classes are not unserialized as `O:notserializable-class`. + ### 5.1.3 - Maintenance release with updated homepage in manifest diff --git a/src/helpers.ts b/src/helpers.ts index dfac809..8fb7d22 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -19,6 +19,17 @@ export function isInteger(value: any): boolean { return typeof value === 'number' && Number.parseInt(value.toString(), 10) === value } +export function isValidKey(key: any): [boolean, string] { + if (!(typeof key === 'string' || typeof key === 'number')) { + return [false, `Invalid key type '${typeof key}' encountered while unserializing`] + } + if (key === '__proto__') { + // Prevent prototype pollution + return [false, 'Key "__proto__" is not allowed'] + } + return [true, ''] +} + export function getIncompleteClass(name: string) { return new __PHP_Incomplete_Class(name) } diff --git a/src/unserialize.ts b/src/unserialize.ts index a76dacf..6a4d4a5 100644 --- a/src/unserialize.ts +++ b/src/unserialize.ts @@ -1,6 +1,6 @@ // eslint-disable-next-line import/no-cycle import Parser from './parser' -import { isInteger, getClass, getIncompleteClass, __PHP_Incomplete_Class, invariant } from './helpers' +import { isInteger, getClass, getIncompleteClass, __PHP_Incomplete_Class, invariant, isValidKey } from './helpers' export type Options = { strict: boolean @@ -71,6 +71,7 @@ function unserializeItem(parser: Parser, scope: Record, options: Op const isArray = pairs.every((item, idx) => isInteger(item.key) && idx === item.key) const result = isArray ? [] : {} pairs.forEach(({ key, value }) => { + invariant(...isValidKey(key)) result[key] = value }) return result @@ -80,10 +81,12 @@ function unserializeItem(parser: Parser, scope: Record, options: Op parser.seekExpected(':') const pairs = parser.getByLength('{', '}', length => unserializePairs(parser, length, scope, options)) const result = getClassReference(name, scope, options.strict) + invariant(!result.unserialize, `Found unserialize method on class ${name} but expected notserializable-class`) const PREFIX_PRIVATE = `\u0000${name}\u0000` const PREFIX_PROTECTED = `\u0000*\u0000` pairs.forEach(({ key, value }) => { + invariant(... isValidKey(key)); if (key.startsWith(PREFIX_PRIVATE)) { // Private field result[key.slice(PREFIX_PRIVATE.length)] = value diff --git a/test/unserialize-test.ts b/test/unserialize-test.ts index b1f7c69..1a9d040 100644 Binary files a/test/unserialize-test.ts and b/test/unserialize-test.ts differ