From 752e9bfd1e44354eec23c18be37ed097327d82c5 Mon Sep 17 00:00:00 2001 From: pmaksimo Date: Mon, 16 Sep 2019 20:06:05 +0100 Subject: [PATCH 1/2] fix: prototype inheritance bug in decodeEncryptionContext If the serialised encryption context contains a key-value pair where the key coincides with a property of Object.prototype, decodeEncryptionContext will incorrectly throw 'Error: Duplicate encryption context key value'. The fix is to create the encryptionContext using Object.create(null) instead of the standard object initialiser. Nota bene: This fix disables the standard calling of Object.prototype functions on encryption contexts returned by decodeEncryptionContext. In particular, it is no longer possible to call encryptionContext.hasOwnProperty(prop). However, due to the fix, one can use the simpler encryptionContext[prop] instead. --- modules/serialize/src/deserialize_factory.ts | 2 +- .../test/deserialize_factory.test.ts | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/modules/serialize/src/deserialize_factory.ts b/modules/serialize/src/deserialize_factory.ts index f9470345d..6f3975c4b 100644 --- a/modules/serialize/src/deserialize_factory.ts +++ b/modules/serialize/src/deserialize_factory.ts @@ -202,7 +202,7 @@ export function deserializeFactory ( * @param encodedEncryptionContext Uint8Array */ function decodeEncryptionContext (encodedEncryptionContext: Uint8Array) { - const encryptionContext: EncryptionContext = {} + const encryptionContext: EncryptionContext = Object.create(null) /* Check for early return (Postcondition): The case of 0 length is defined as an empty object. */ if (!encodedEncryptionContext.byteLength) { return encryptionContext diff --git a/modules/serialize/test/deserialize_factory.test.ts b/modules/serialize/test/deserialize_factory.test.ts index 1e5c5850d..0888efee0 100644 --- a/modules/serialize/test/deserialize_factory.test.ts +++ b/modules/serialize/test/deserialize_factory.test.ts @@ -78,6 +78,29 @@ describe('deserializeFactory:decodeEncryptionContext', () => { expect(test).to.have.property('information') .and.to.eql('\u00bd + \u00bc = \u00be') }) + + it('Keys may be properties of Object.prototype, decodeEncryptionContext has to succeed', () => { + const { decodeEncryptionContext } = deserializeFactory(toUtf8, WebCryptoAlgorithmSuite) + + /* Single key-value pair */ + const kvPairCount = Buffer.alloc(2) + kvPairCount.writeUInt16BE(1, 0) + + /* Actual key-value pair */ + const kvPair = [ 'hasOwnProperty', 'arbitraryValue' ] + + /* Constructing the encryption context */ + const kvPairBinary = kvPair.map(str => new Uint8Array([...Buffer.from(str)])) + const encryptionContext = concatBuffers(kvPairCount, ...kvPairBinary.map(bufStr => { + const len = Buffer.alloc(2) + len.writeUInt16BE(bufStr.byteLength, 0) + return concatBuffers(len, bufStr) + })) + + const test = decodeEncryptionContext(encryptionContext) + expect(test).to.have.property('hasOwnProperty') + .and.to.eql('arbitraryValue') + }) }) describe('deserializeFactory:deserializeEncryptedDataKeys', () => { From 28d57e63fefc125783b5fd64338b5116e5fe9aa6 Mon Sep 17 00:00:00 2001 From: pmaksimo Date: Wed, 18 Sep 2019 19:12:58 +0100 Subject: [PATCH 2/2] fix: prototype inheritance bug in decodeEncryptionContext, additional test vector in fixtures.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the serialised encryption context contains a key-value pair where the key coincides with a property of Object.prototype, decodeEncryptionContext will incorrectly throw 'Error: Duplicate encryption context key value'. The fix is to create the encryptionContext using Object.create(null) instead of the standard object initialiser. This commit also adds a test vector in fixtures.ts, containing ‘hasOwnProperty’ as its only key. Nota bene: This fix disables the standard calling of Object.prototype functions on encryption contexts returned by decodeEncryptionContext. In particular, it is no longer possible to call encryptionContext.hasOwnProperty(prop). However, due to the fix, one can use the simpler encryptionContext[prop] instead. --- .../serialize/test/deserialize_factory.test.ts | 16 ++-------------- modules/serialize/test/fixtures.ts | 4 ++++ 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/modules/serialize/test/deserialize_factory.test.ts b/modules/serialize/test/deserialize_factory.test.ts index 0888efee0..c0bf67e18 100644 --- a/modules/serialize/test/deserialize_factory.test.ts +++ b/modules/serialize/test/deserialize_factory.test.ts @@ -82,20 +82,8 @@ describe('deserializeFactory:decodeEncryptionContext', () => { it('Keys may be properties of Object.prototype, decodeEncryptionContext has to succeed', () => { const { decodeEncryptionContext } = deserializeFactory(toUtf8, WebCryptoAlgorithmSuite) - /* Single key-value pair */ - const kvPairCount = Buffer.alloc(2) - kvPairCount.writeUInt16BE(1, 0) - - /* Actual key-value pair */ - const kvPair = [ 'hasOwnProperty', 'arbitraryValue' ] - - /* Constructing the encryption context */ - const kvPairBinary = kvPair.map(str => new Uint8Array([...Buffer.from(str)])) - const encryptionContext = concatBuffers(kvPairCount, ...kvPairBinary.map(bufStr => { - const len = Buffer.alloc(2) - len.writeUInt16BE(bufStr.byteLength, 0) - return concatBuffers(len, bufStr) - })) + /* hasOwnProperty test vector */ + const encryptionContext = fixtures.hasOwnPropertyEncryptionContext().slice(2) const test = decodeEncryptionContext(encryptionContext) expect(test).to.have.property('hasOwnProperty') diff --git a/modules/serialize/test/fixtures.ts b/modules/serialize/test/fixtures.ts index 12efbc33e..7ce264ace 100644 --- a/modules/serialize/test/fixtures.ts +++ b/modules/serialize/test/fixtures.ts @@ -77,6 +77,10 @@ export function duplicateKeysEncryptionContext () { return new Uint8Array([ 0, 43, 0, 4, 0, 11, 105, 110, 102, 111, 114, 109, 97, 116, 105, 111, 110, 0, 12, 194, 189, 32, 43, 32, 194, 188, 32, 61, 32, 194, 190, 0, 11, 105, 110, 102, 111, 114, 109, 97, 116, 105, 111, 110, 0, 12, 194, 189, 32, 43, 32, 194, 188, 32, 61, 32, 194, 190, 0, 4, 115, 111, 109, 101, 0, 6, 112, 117, 98, 108, 105, 99, 0, 4, 115, 111, 109, 101, 0, 6, 112, 117, 98, 108, 105, 99 ]) } +export function hasOwnPropertyEncryptionContext () { + return new Uint8Array([ 0, 34, 0, 1, 0, 14, 104, 97, 115, 79, 119, 110, 80, 114, 111, 112, 101, 114, 116, 121, 0, 14, 97, 114, 98, 105, 116, 114, 97, 114, 121, 86, 97, 108, 117, 101 ]) +} + export function basicFrameIV () { return new Uint8Array([0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1]) }