fix(ref-imp): #873 - Allowed only canonically encoded DID suffix to p…

…ass hash check.
decentralized-identity · Oct 2, 2020 · 7b3be64 · 7b3be64
1 parent 3eaf265
commit 7b3be64
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 2 deletions.
diff --git a/lib/bitcoin/BitcoinClient.ts b/lib/bitcoin/BitcoinClient.ts
@@ -595,7 +595,7 @@ export default class BitcoinClient {
     previousFreezeDurationInBlocks: number): Promise<Transaction> {
 
     // tslint:disable-next-line: max-line-length
-    console.info(`Creating a transaction to return (to the wallet) the preivously frozen amount from transaction with id: ${previousFreezeTransaction.id} which was frozen for block duration: ${previousFreezeDurationInBlocks}`);
+    console.info(`Creating a transaction to return (to the wallet) the previously frozen amount from transaction with id: ${previousFreezeTransaction.id} which was frozen for block duration: ${previousFreezeDurationInBlocks}`);
 
     return this.createSpendTransactionFromFrozenTransaction(
       previousFreezeTransaction,

diff --git a/lib/core/versions/latest/Multihash.ts b/lib/core/versions/latest/Multihash.ts
@@ -201,7 +201,11 @@ export default class Multihash {
 
       const actualMultihashBuffer = Multihash.hash(content, hashAlgorithmCode);
 
-      return Buffer.compare(actualMultihashBuffer, expectedMultihashBuffer) === 0;
+      // Compare the strings instead of buffers, because encoding schemes such as base64URL can allow two distinct strings to decode into the same buffer.
+      // e.g. 'EiAJID5-y7rbEs7I3PPiMtwVf28LTkPFD4BWIZPCtb6AMg' and
+      //      'EiAJID5-y7rbEs7I3PPiMtwVf28LTkPFD4BWIZPCtb6AMv' would decode into the same buffer.
+      const actualMultihashString = Encoder.encode(actualMultihashBuffer);
+      return actualMultihashString === encodedMultihash;
     } catch (error) {
       console.log(error);
       return false;

diff --git a/tests/core/Multihash.spec.ts b/tests/core/Multihash.spec.ts
@@ -84,6 +84,23 @@ describe('Multihash', async () => {
 
       expect(validHash).toBeFalsy();
     });
+
+    it('should return false if given encoded multihash is not using the canonical encoding.', async () => {
+      const anyContent = Buffer.from('any content');
+
+      // Canonical encoded multihash of 'any content' is 'EiDDidVHVekuMIYV3HI5nfp8KP6s3_W44Pd-MO5b-XK5iQ'
+      const defaultContentEncodedMultihash = 'EiDDidVHVekuMIYV3HI5nfp8KP6s3_W44Pd-MO5b-XK5iQ';
+      const modifiedContentEncodedMultihash = 'EiDDidVHVekuMIYV3HI5nfp8KP6s3_W44Pd-MO5b-XK5iR';
+
+      // Two multihash strings decodes into the same buffer.
+      expect(Encoder.decodeAsBuffer(defaultContentEncodedMultihash)).toEqual(Encoder.decodeAsBuffer(modifiedContentEncodedMultihash));
+
+      const validHashCheckResult = (Multihash as any).verify(anyContent, defaultContentEncodedMultihash);
+      const invalidHashCheckResult = (Multihash as any).verify(anyContent, modifiedContentEncodedMultihash);
+
+      expect(validHashCheckResult).toBeTruthy();
+      expect(invalidHashCheckResult).toBeFalsy();
+    });
   });
 
   describe('verifyDoubleHash()', async () => {