From 95d5edf5c4f8a1cd6c23deaac3a446959fde779e Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 13 Dec 2022 09:16:47 -0500 Subject: [PATCH] feat(NODE-1921)!: validate serializer root input (#537) --- docs/upgrade-to-v5.md | 21 +++++ src/bson.ts | 5 +- src/parser/serializer.ts | 119 +++++++++++++++++++-------- test/node/bson_test.js | 4 +- test/node/circular_reference.test.ts | 110 +++++++++++++++++++++++++ test/node/detect_cyclic_dep_tests.js | 57 ------------- test/node/extended_json.test.ts | 48 ----------- test/node/parser/serializer.test.ts | 75 +++++++++++++++++ 8 files changed, 298 insertions(+), 141 deletions(-) create mode 100644 test/node/circular_reference.test.ts delete mode 100644 test/node/detect_cyclic_dep_tests.js diff --git a/docs/upgrade-to-v5.md b/docs/upgrade-to-v5.md index 75146805..acce5c12 100644 --- a/docs/upgrade-to-v5.md +++ b/docs/upgrade-to-v5.md @@ -217,3 +217,24 @@ iLoveJavascript(); // prints "I love javascript" // iLoveJavascript.name === "iLoveJavascript" ``` + +### `BSON.serialize()` validation + +The BSON format does not support encoding arrays as the **root** object. +However, in javascript arrays are just objects where the keys are numeric (and a magic `length` property), so round tripping an array (ex. `[1, 2]`) though BSON would return `{ '0': 1, '1': 2 }`. + +`BSON.serialize()` now validates input types, the input to serialize must be an object or a `Map`, arrays will now cause an error. + +```typescript +BSON.serialize([1, 2, 3]) +// BSONError: serialize does not support an array as the root input +``` + +if the functionality of turning arrays into an object with numeric keys is useful see the following example: + +```typescript +// Migration example: +const result = BSON.serialize(Object.fromEntries([1, true, 'blue'].entries())) +BSON.deserialize(result) +// { '0': 1, '1': true, '2': 'blue' } +``` diff --git a/src/bson.ts b/src/bson.ts index fd012a33..066fc63d 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -109,7 +109,7 @@ export function serialize(object: Document, options: SerializeOptions = {}): Uin 0, serializeFunctions, ignoreUndefined, - [] + null ); // Create the final buffer @@ -152,7 +152,8 @@ export function serializeWithBufferAndIndex( 0, 0, serializeFunctions, - ignoreUndefined + ignoreUndefined, + null ); finalBuffer.set(buffer.subarray(0, serializationIndex), startIndex); diff --git a/src/parser/serializer.ts b/src/parser/serializer.ts index 918f51b6..5d1a7d4a 100644 --- a/src/parser/serializer.ts +++ b/src/parser/serializer.ts @@ -13,7 +13,15 @@ import type { MinKey } from '../min_key'; import type { ObjectId } from '../objectid'; import type { BSONRegExp } from '../regexp'; import { ByteUtils } from '../utils/byte_utils'; -import { isBigInt64Array, isBigUInt64Array, isDate, isMap, isRegExp, isUint8Array } from './utils'; +import { + isAnyArrayBuffer, + isBigInt64Array, + isBigUInt64Array, + isDate, + isMap, + isRegExp, + isUint8Array +} from './utils'; /** @public */ export interface SerializeOptions { @@ -270,18 +278,18 @@ function serializeObject( key: string, value: Document, index: number, - checkKeys = false, - depth = 0, - serializeFunctions = false, - ignoreUndefined = true, - path: Document[] = [] + checkKeys: boolean, + depth: number, + serializeFunctions: boolean, + ignoreUndefined: boolean, + path: Set ) { - for (let i = 0; i < path.length; i++) { - if (path[i] === value) throw new BSONError('cyclic dependency detected'); + if (path.has(value)) { + throw new BSONError('Cannot convert circular structure to BSON'); } - // Push value to stack - path.push(value); + path.add(value); + // Write the type buffer[index++] = Array.isArray(value) ? constants.BSON_DATA_ARRAY : constants.BSON_DATA_OBJECT; // Number of written bytes @@ -299,8 +307,9 @@ function serializeObject( ignoreUndefined, path ); - // Pop stack - path.pop(); + + path.delete(value); + return endIndex; } @@ -410,7 +419,8 @@ function serializeCode( checkKeys = false, depth = 0, serializeFunctions = false, - ignoreUndefined = true + ignoreUndefined = true, + path: Set ) { if (value.scope && typeof value.scope === 'object') { // Write the type @@ -441,7 +451,6 @@ function serializeCode( // Write the index = index + codeSize + 4; - // // Serialize the scope value const endIndex = serializeInto( buffer, @@ -450,7 +459,8 @@ function serializeCode( index, depth + 1, serializeFunctions, - ignoreUndefined + ignoreUndefined, + path ); index = endIndex - 1; @@ -555,7 +565,8 @@ function serializeDBRef( value: DBRef, index: number, depth: number, - serializeFunctions: boolean + serializeFunctions: boolean, + path: Set ) { // Write the type buffer[index++] = constants.BSON_DATA_OBJECT; @@ -577,7 +588,16 @@ function serializeDBRef( } output = Object.assign(output, value.fields); - const endIndex = serializeInto(buffer, output, false, index, depth + 1, serializeFunctions); + const endIndex = serializeInto( + buffer, + output, + false, + index, + depth + 1, + serializeFunctions, + true, + path + ); // Calculate object size const size = endIndex - startIndex; @@ -593,18 +613,48 @@ function serializeDBRef( export function serializeInto( buffer: Uint8Array, object: Document, - checkKeys = false, - startingIndex = 0, - depth = 0, - serializeFunctions = false, - ignoreUndefined = true, - path: Document[] = [] + checkKeys: boolean, + startingIndex: number, + depth: number, + serializeFunctions: boolean, + ignoreUndefined: boolean, + path: Set | null ): number { - startingIndex = startingIndex || 0; - path = path || []; + if (path == null) { + // We are at the root input + if (object == null) { + // ONLY the root should turn into an empty document + // BSON Empty document has a size of 5 (LE) + buffer[0] = 0x05; + buffer[1] = 0x00; + buffer[2] = 0x00; + buffer[3] = 0x00; + // All documents end with null terminator + buffer[4] = 0x00; + return 5; + } + + if (Array.isArray(object)) { + throw new BSONError('serialize does not support an array as the root input'); + } + if (typeof object !== 'object') { + throw new BSONError('serialize does not support non-object as the root input'); + } else if ('_bsontype' in object && typeof object._bsontype === 'string') { + throw new BSONError(`BSON types cannot be serialized as a document`); + } else if ( + isDate(object) || + isRegExp(object) || + isUint8Array(object) || + isAnyArrayBuffer(object) + ) { + throw new BSONError(`date, regexp, typedarray, and arraybuffer cannot be BSON documents`); + } + + path = new Set(); + } // Push the object to the path - path.push(object); + path.add(object); // Start place to serialize into let index = startingIndex + 4; @@ -674,14 +724,15 @@ export function serializeInto( checkKeys, depth, serializeFunctions, - ignoreUndefined + ignoreUndefined, + path ); } else if (value['_bsontype'] === 'Binary') { index = serializeBinary(buffer, key, value, index); } else if (value['_bsontype'] === 'Symbol') { index = serializeSymbol(buffer, key, value, index); } else if (value['_bsontype'] === 'DBRef') { - index = serializeDBRef(buffer, key, value, index, depth, serializeFunctions); + index = serializeDBRef(buffer, key, value, index, depth, serializeFunctions, path); } else if (value['_bsontype'] === 'BSONRegExp') { index = serializeBSONRegExp(buffer, key, value, index); } else if (value['_bsontype'] === 'Int32') { @@ -772,7 +823,8 @@ export function serializeInto( checkKeys, depth, serializeFunctions, - ignoreUndefined + ignoreUndefined, + path ); } else if (typeof value === 'function' && serializeFunctions) { index = serializeFunction(buffer, key, value, index); @@ -781,7 +833,7 @@ export function serializeInto( } else if (value['_bsontype'] === 'Symbol') { index = serializeSymbol(buffer, key, value, index); } else if (value['_bsontype'] === 'DBRef') { - index = serializeDBRef(buffer, key, value, index, depth, serializeFunctions); + index = serializeDBRef(buffer, key, value, index, depth, serializeFunctions, path); } else if (value['_bsontype'] === 'BSONRegExp') { index = serializeBSONRegExp(buffer, key, value, index); } else if (value['_bsontype'] === 'Int32') { @@ -876,7 +928,8 @@ export function serializeInto( checkKeys, depth, serializeFunctions, - ignoreUndefined + ignoreUndefined, + path ); } else if (typeof value === 'function' && serializeFunctions) { index = serializeFunction(buffer, key, value, index); @@ -885,7 +938,7 @@ export function serializeInto( } else if (value['_bsontype'] === 'Symbol') { index = serializeSymbol(buffer, key, value, index); } else if (value['_bsontype'] === 'DBRef') { - index = serializeDBRef(buffer, key, value, index, depth, serializeFunctions); + index = serializeDBRef(buffer, key, value, index, depth, serializeFunctions, path); } else if (value['_bsontype'] === 'BSONRegExp') { index = serializeBSONRegExp(buffer, key, value, index); } else if (value['_bsontype'] === 'Int32') { @@ -899,7 +952,7 @@ export function serializeInto( } // Remove the path - path.pop(); + path.delete(object); // Final padding byte for object buffer[index++] = 0x00; diff --git a/test/node/bson_test.js b/test/node/bson_test.js index fe1a07d1..b71c4d6e 100644 --- a/test/node/bson_test.js +++ b/test/node/bson_test.js @@ -1849,7 +1849,9 @@ describe('BSON', function () { // Array const array = [new ObjectIdv400(), new OldObjectID(), new ObjectId()]; - const deserializedArrayAsMap = BSON.deserialize(BSON.serialize(array)); + const deserializedArrayAsMap = BSON.deserialize( + BSON.serialize(Object.fromEntries(array.entries())) + ); const deserializedArray = Object.keys(deserializedArrayAsMap).map( x => deserializedArrayAsMap[x] ); diff --git a/test/node/circular_reference.test.ts b/test/node/circular_reference.test.ts new file mode 100644 index 00000000..e70fe192 --- /dev/null +++ b/test/node/circular_reference.test.ts @@ -0,0 +1,110 @@ +import { expect } from 'chai'; +import * as BSON from '../register-bson'; +import type { Code, Document } from '../..'; +import { inspect, types } from 'node:util'; +import { DBRef } from '../register-bson'; + +const EJSON = BSON.EJSON; + +function setOn(object: Document | unknown[] | Map, value: unknown) { + if (Array.isArray(object)) { + object[Math.floor(Math.random() * object.length)] = value; + } else if (types.isMap(object)) { + // @ts-expect-error: "readonly" map case does not apply + object.set('a', value); + } else { + object.a = value; + } +} + +function* generateTests() { + for (const makeRoot of [() => new Map(), () => ({})]) { + for (const makeNestedType of [() => new Map(), () => ({}), () => []]) { + const root = makeRoot(); + const nested = makeNestedType(); + setOn(root, nested); + setOn(nested, root); + + yield { + title: `root that is a ${types.isMap(root) ? 'map' : 'object'} with a nested ${ + types.isMap(nested) ? 'map' : Array.isArray(nested) ? 'array' : 'object' + } with a circular reference to the root throws`, + input: root + }; + } + } +} + +describe('Cyclic reference detection', () => { + context('fuzz BSON circular references', () => { + for (const test of generateTests()) { + it(test.title, () => { + expect(() => BSON.serialize(test.input), inspect(test.input)).to.throw(/circular/); + }); + } + }); + + context('in Code with scope', () => { + it('throws if code.scope is circular', () => { + const root: { code: Code | null } = { code: null }; + root.code = new BSON.Code('function() {}', { a: root }); + expect(() => BSON.serialize(root)).to.throw(/circular/); + }); + }); + + context('in DBRef with fields', () => { + it('throws if dbref.fields is circular', () => { + const root: { dbref: DBRef | null } = { dbref: null }; + root.dbref = new BSON.DBRef('test', new BSON.ObjectId(), 'test', { a: root }); + expect(() => BSON.serialize(root)).to.throw(/circular/); + }); + }); + + context('EJSON circular references', () => { + it('should throw a helpful error message for input with circular references', function () { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const obj: any = { + some: { + property: { + array: [] + } + } + }; + obj.some.property.array.push(obj.some); + expect(() => EJSON.serialize(obj)).to.throw(`\ +Converting circular structure to EJSON: + (root) -> some -> property -> array -> index 0 + \\-----------------------------/`); + }); + + it('should throw a helpful error message for input with circular references, one-level nested', function () { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const obj: any = {}; + obj.obj = obj; + expect(() => EJSON.serialize(obj)).to.throw(`\ +Converting circular structure to EJSON: + (root) -> obj + \\-------/`); + }); + + it('should throw a helpful error message for input with circular references, one-level nested inside base object', function () { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const obj: any = {}; + obj.obj = obj; + expect(() => EJSON.serialize({ foo: obj })).to.throw(`\ +Converting circular structure to EJSON: + (root) -> foo -> obj + \\------/`); + }); + + it('should throw a helpful error message for input with circular references, pointing back to base object', function () { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const obj: any = { foo: {} }; + obj.foo.obj = obj; + expect(() => EJSON.serialize(obj)).to.throw(`\ +Converting circular structure to EJSON: + (root) -> foo -> obj + \\--------------/`); + }); + }); +}); diff --git a/test/node/detect_cyclic_dep_tests.js b/test/node/detect_cyclic_dep_tests.js deleted file mode 100644 index 0c7342aa..00000000 --- a/test/node/detect_cyclic_dep_tests.js +++ /dev/null @@ -1,57 +0,0 @@ -'use strict'; - -const BSON = require('../register-bson'); - -describe('Cyclic Dependencies', function () { - /** - * @ignore - */ - it('Should correctly detect cyclic dependency in nested objects', function (done) { - // Force cyclic dependency - var a = { b: {} }; - a.b.c = a; - try { - // Attempt to serialize cyclic dependency - BSON.serialize(a); - } catch (err) { - expect('cyclic dependency detected').to.equal(err.message); - } - - done(); - }); - - /** - * @ignore - */ - it('Should correctly detect cyclic dependency in deeploy nested objects', function (done) { - // Force cyclic dependency - var a = { b: { c: [{ d: {} }] } }; - a.b.c[0].d.a = a; - - try { - // Attempt to serialize cyclic dependency - BSON.serialize(a); - } catch (err) { - expect('cyclic dependency detected').to.equal(err.message); - } - - done(); - }); - - /** - * @ignore - */ - it('Should correctly detect cyclic dependency in nested array', function (done) { - // Force cyclic dependency - var a = { b: {} }; - a.b.c = [a]; - try { - // Attempt to serialize cyclic dependency - BSON.serialize(a); - } catch (err) { - expect('cyclic dependency detected').to.equal(err.message); - } - - done(); - }); -}); diff --git a/test/node/extended_json.test.ts b/test/node/extended_json.test.ts index 4c12100a..4a372088 100644 --- a/test/node/extended_json.test.ts +++ b/test/node/extended_json.test.ts @@ -530,54 +530,6 @@ describe('Extended JSON', function () { expect(deserialized.__proto__.a).to.equal(42); }); - context('circular references', () => { - it('should throw a helpful error message for input with circular references', function () { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const obj: any = { - some: { - property: { - array: [] - } - } - }; - obj.some.property.array.push(obj.some); - expect(() => EJSON.serialize(obj)).to.throw(`\ -Converting circular structure to EJSON: - (root) -> some -> property -> array -> index 0 - \\-----------------------------/`); - }); - - it('should throw a helpful error message for input with circular references, one-level nested', function () { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const obj: any = {}; - obj.obj = obj; - expect(() => EJSON.serialize(obj)).to.throw(`\ -Converting circular structure to EJSON: - (root) -> obj - \\-------/`); - }); - - it('should throw a helpful error message for input with circular references, one-level nested inside base object', function () { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const obj: any = {}; - obj.obj = obj; - expect(() => EJSON.serialize({ foo: obj })).to.throw(`\ -Converting circular structure to EJSON: - (root) -> foo -> obj - \\------/`); - }); - - it('should throw a helpful error message for input with circular references, pointing back to base object', function () { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const obj: any = { foo: {} }; - obj.foo.obj = obj; - expect(() => EJSON.serialize(obj)).to.throw(`\ -Converting circular structure to EJSON: - (root) -> foo -> obj - \\--------------/`); - }); - }); - context('when dealing with legacy extended json', function () { describe('.stringify', function () { context('when serializing binary', function () { diff --git a/test/node/parser/serializer.test.ts b/test/node/parser/serializer.test.ts index 10b839ed..352fc538 100644 --- a/test/node/parser/serializer.test.ts +++ b/test/node/parser/serializer.test.ts @@ -14,4 +14,79 @@ describe('serialize()', () => { ]) ); }); + + it('returns an empty document when the root input is nullish', () => { + // @ts-expect-error: Testing nullish input, not supported by type defs but code works gracefully + const emptyDocumentUndef = BSON.serialize(undefined); + expect(emptyDocumentUndef).to.deep.equal(new Uint8Array([5, 0, 0, 0, 0])); + // @ts-expect-error: Testing nullish input, not supported by type defs but code works gracefully + const emptyDocumentNull = BSON.serialize(null); + expect(emptyDocumentNull).to.deep.equal(new Uint8Array([5, 0, 0, 0, 0])); + }); + + it('does not turn nested nulls into empty documents', () => { + const nestedNull = bufferFromHexArray([ + '0A', // null type + '6100', // 'a\x00' + '' // null is encoded as nothing + ]); + const emptyDocumentUndef = BSON.serialize({ a: undefined }, { ignoreUndefined: false }); + expect(emptyDocumentUndef).to.deep.equal(nestedNull); + const emptyDocumentNull = BSON.serialize({ a: null }); + expect(emptyDocumentNull).to.deep.equal(nestedNull); + }); + + describe('validates input types', () => { + it('does not permit arrays as the root input', () => { + expect(() => BSON.serialize([])).to.throw(/does not support an array/); + }); + + it('does not permit objects with a _bsontype string to be serialized at the root', () => { + expect(() => BSON.serialize({ _bsontype: 'iLoveJavascript' })).to.throw(/BSON types cannot/); + // a nested invalid _bsontype throws something different + expect(() => BSON.serialize({ a: { _bsontype: 'iLoveJavascript' } })).to.throw( + /invalid _bsontype/ + ); + }); + + it('does permit objects with a _bsontype prop that is not a string', () => { + const expected = bufferFromHexArray([ + '10', // int32 + Buffer.from('_bsontype\x00', 'utf8').toString('hex'), + '02000000' + ]); + const result = BSON.serialize({ _bsontype: 2 }); + expect(result).to.deep.equal(expected); + + expect(() => BSON.serialize({ _bsontype: true })).to.not.throw(); + expect(() => BSON.serialize({ _bsontype: /a/ })).to.not.throw(); + expect(() => BSON.serialize({ _bsontype: new Date() })).to.not.throw(); + }); + + it('does not permit non-objects as the root input', () => { + // @ts-expect-error: Testing invalid input + expect(() => BSON.serialize(true)).to.throw(/does not support non-object/); + // @ts-expect-error: Testing invalid input + expect(() => BSON.serialize(2)).to.throw(/does not support non-object/); + // @ts-expect-error: Testing invalid input + expect(() => BSON.serialize(2n)).to.throw(/does not support non-object/); + // @ts-expect-error: Testing invalid input + expect(() => BSON.serialize(Symbol())).to.throw(/does not support non-object/); + // @ts-expect-error: Testing invalid input + expect(() => BSON.serialize('')).to.throw(/does not support non-object/); + expect(() => + BSON.serialize(function () { + // ignore + }) + ).to.throw(/does not support non-object/); + }); + + it('does not permit certain objects that are typically values as the root input', () => { + expect(() => BSON.serialize(new Date())).to.throw(/cannot be BSON documents/); + expect(() => BSON.serialize(/a/)).to.throw(/cannot be BSON documents/); + expect(() => BSON.serialize(new ArrayBuffer(2))).to.throw(/cannot be BSON documents/); + expect(() => BSON.serialize(Buffer.alloc(2))).to.throw(/cannot be BSON documents/); + expect(() => BSON.serialize(new Uint8Array(3))).to.throw(/cannot be BSON documents/); + }); + }); });