From bdb14f91a83c352f2b975d9252f35f20e176116a Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Mon, 1 Jun 2020 10:27:08 +0200 Subject: [PATCH] Pre-checks on verify input lengths (#608) * Pre-checks on verify input lengths * Small cleanup * Update error signatures --- CHANGELOG.md | 1 + packages/util-crypto/src/address/decode.ts | 3 +-- packages/util-crypto/src/address/encode.ts | 1 - packages/util-crypto/src/key/extractPath.ts | 2 +- packages/util-crypto/src/key/fromPath.ts | 12 +++++----- packages/util-crypto/src/nacl/sign.ts | 2 +- packages/util-crypto/src/nacl/verify.spec.ts | 22 ++++++++++++++++++- packages/util-crypto/src/nacl/verify.ts | 9 +++++--- packages/util-crypto/src/schnorrkel/sign.ts | 4 ++-- .../src/schnorrkel/signVerify.spec.ts | 20 +++++++++++++++++ packages/util-crypto/src/schnorrkel/verify.ts | 7 ++++-- packages/util-crypto/src/signature/verify.ts | 5 ++--- 12 files changed, 67 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 518769f22..cf1e211ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - **Important** Removed `mnemonicToSeed`, it is only for bip39, use `mnemonicToMiniSecret` for the Substrate variant - Added `u8aEq` to compare two Uint8Array or hex values for an exact match - Added `addressEq` to compare two addresses (ss58, Uint8array or hex) for an exact match +- Throw error on `{nacl,schnorrkel}Verify` on invalid input lengths - Performance optimizations in `u8aToHex` & `u8aConcat` ## 2.11.1 May 26, 2020 diff --git a/packages/util-crypto/src/address/decode.ts b/packages/util-crypto/src/address/decode.ts index 54d9560a4..ec9b2cfc3 100644 --- a/packages/util-crypto/src/address/decode.ts +++ b/packages/util-crypto/src/address/decode.ts @@ -19,8 +19,7 @@ export default function decode (encoded: string | Uint8Array, ignoreChecksum?: b } const decoded = bufferToU8a(bs58.decode(encoded)); - const error = (message: string): string => - `Decoding ${encoded as string}: ${message}`; + const error = (message: string) => `Decoding ${encoded as string}: ${message}`; // assert(defaults.allowedPrefix.includes(decoded[0] as Prefix), error('Invalid decoded address prefix')); assert(defaults.allowedEncodedLengths.includes(decoded.length), error('Invalid decoded address length')); diff --git a/packages/util-crypto/src/address/encode.ts b/packages/util-crypto/src/address/encode.ts index 15efb990a..95e9b84a0 100644 --- a/packages/util-crypto/src/address/encode.ts +++ b/packages/util-crypto/src/address/encode.ts @@ -20,7 +20,6 @@ export default function encode (_key: Uint8Array | string, ss58Format: Prefix = assert(defaults.allowedDecodedLengths.includes(key.length), `Expected a valid key to convert, with length ${defaults.allowedDecodedLengths.join(', ')}`); const isPublicKey = key.length === 32; - const input = u8aConcat(new Uint8Array([ss58Format]), key); const hash = sshash(input); diff --git a/packages/util-crypto/src/key/extractPath.ts b/packages/util-crypto/src/key/extractPath.ts index a66e34e5b..bbad57fb3 100644 --- a/packages/util-crypto/src/key/extractPath.ts +++ b/packages/util-crypto/src/key/extractPath.ts @@ -13,7 +13,7 @@ export interface ExtractResult { } /** - * @description Extract derivation juntions from the supplied path + * @description Extract derivation junctions from the supplied path */ export default function keyExtractPath (derivePath: string): ExtractResult { const parts = derivePath.match(RE_JUNCTION); diff --git a/packages/util-crypto/src/key/fromPath.ts b/packages/util-crypto/src/key/fromPath.ts index 2cb3e4811..aef7eca2a 100644 --- a/packages/util-crypto/src/key/fromPath.ts +++ b/packages/util-crypto/src/key/fromPath.ts @@ -9,12 +9,14 @@ import keyHdkdEd15519 from './hdkdEd25519'; import keyHdkdSr15519 from './hdkdSr25519'; import keyHdkdEcdsa from './hdkdEcdsa'; +const generators = { + ecdsa: keyHdkdEcdsa, + ed25519: keyHdkdEd15519, + sr25519: keyHdkdSr15519 +}; + export default function keyFromPath (pair: Keypair, path: DeriveJunction[], type: KeypairType): Keypair { - const keyHdkd = { - ecdsa: keyHdkdEcdsa, - ed25519: keyHdkdEd15519, - sr25519: keyHdkdSr15519 - }[type]; + const keyHdkd = generators[type]; return path.reduce((pair, junction): Keypair => { return keyHdkd(pair, junction); diff --git a/packages/util-crypto/src/nacl/sign.ts b/packages/util-crypto/src/nacl/sign.ts index 5a1f327da..f2e546135 100644 --- a/packages/util-crypto/src/nacl/sign.ts +++ b/packages/util-crypto/src/nacl/sign.ts @@ -23,7 +23,7 @@ import { isReady, ed25519Sign } from '@polkadot/wasm-crypto'; * ``` */ export default function naclSign (message: Uint8Array | string, { publicKey, secretKey }: Partial): Uint8Array { - assert(secretKey, 'Expected valid secretKey'); + assert(secretKey, 'Expected a valid secretKey'); const messageU8a = u8aToU8a(message); diff --git a/packages/util-crypto/src/nacl/verify.spec.ts b/packages/util-crypto/src/nacl/verify.spec.ts index 13bbe3dda..66e5316a5 100644 --- a/packages/util-crypto/src/nacl/verify.spec.ts +++ b/packages/util-crypto/src/nacl/verify.spec.ts @@ -7,7 +7,7 @@ import { waitReady } from '@polkadot/wasm-crypto'; import { naclVerify, naclKeypairFromSeed } from '.'; -describe('naclSign', (): void => { +describe('naclVerify', (): void => { let publicKey: Uint8Array; let signature: Uint8Array; @@ -51,4 +51,24 @@ describe('naclSign', (): void => { ) ).toEqual(false); }); + + it('throws error when publicKey lengths do not match', (): void => { + expect( + () => naclVerify( + new Uint8Array([0x61, 0x62, 0x63, 0x64]), + signature, + new Uint8Array([1, 2]) + ) + ).toThrow(/Invalid publicKey/); + }); + + it('throws error when signature lengths do not match', (): void => { + expect( + () => naclVerify( + new Uint8Array([0x61, 0x62, 0x63, 0x64]), + new Uint8Array([1, 2]), + publicKey + ) + ).toThrow(/Invalid signature/); + }); }); diff --git a/packages/util-crypto/src/nacl/verify.ts b/packages/util-crypto/src/nacl/verify.ts index 9f5d613fd..0dfac7cca 100644 --- a/packages/util-crypto/src/nacl/verify.ts +++ b/packages/util-crypto/src/nacl/verify.ts @@ -3,14 +3,14 @@ // of the Apache-2.0 license. See the LICENSE file for details. import nacl from 'tweetnacl'; -import { u8aToU8a } from '@polkadot/util'; +import { assert, u8aToU8a } from '@polkadot/util'; import { isReady, ed25519Verify } from '@polkadot/wasm-crypto'; /** * @name naclSign * @summary Verifies the signature on the supplied message. * @description - * Verifies the `signature` on `message` with the supplied `plublicKey`. Returns `true` on sucess, `false` otherwise. + * Verifies the `signature` on `message` with the supplied `publicKey`. Returns `true` on sucess, `false` otherwise. * @example *
* @@ -22,8 +22,11 @@ import { isReady, ed25519Verify } from '@polkadot/wasm-crypto'; */ export default function naclVerify (message: Uint8Array | string, signature: Uint8Array | string, publicKey: Uint8Array | string): boolean { const messageU8a = u8aToU8a(message); - const signatureU8a = u8aToU8a(signature); const publicKeyU8a = u8aToU8a(publicKey); + const signatureU8a = u8aToU8a(signature); + + assert(publicKeyU8a.length === 32, `Invalid publicKey, received ${publicKeyU8a.length}, expected 32`); + assert(signatureU8a.length === 64, `Invalid signature, received ${signatureU8a.length} bytes, expected 64`); return isReady() ? ed25519Verify(signatureU8a, messageU8a, publicKeyU8a) diff --git a/packages/util-crypto/src/schnorrkel/sign.ts b/packages/util-crypto/src/schnorrkel/sign.ts index aae40f32d..6d8053dbe 100644 --- a/packages/util-crypto/src/schnorrkel/sign.ts +++ b/packages/util-crypto/src/schnorrkel/sign.ts @@ -14,8 +14,8 @@ import { sr25519Sign } from '@polkadot/wasm-crypto'; * @description Returns message signature of `message`, using the supplied pair */ export default function schnorrkelSign (message: Uint8Array | string, { publicKey, secretKey }: Partial): Uint8Array { - assert(publicKey?.length === 32, 'Expected valid publicKey, 32-bytes'); - assert(secretKey?.length === 64, 'Expected valid secretKey, 64-bytes'); + assert(publicKey?.length === 32, 'Expected a valid publicKey, 32-bytes'); + assert(secretKey?.length === 64, 'Expected a valid secretKey, 64-bytes'); const messageU8a = u8aToU8a(message); diff --git a/packages/util-crypto/src/schnorrkel/signVerify.spec.ts b/packages/util-crypto/src/schnorrkel/signVerify.spec.ts index 82c3f0967..9b959c190 100644 --- a/packages/util-crypto/src/schnorrkel/signVerify.spec.ts +++ b/packages/util-crypto/src/schnorrkel/signVerify.spec.ts @@ -29,4 +29,24 @@ describe('sign and verify', (): void => { expect(verify(MESSAGE, signature, pair.publicKey)).toBe(true); }); + + it('throws error when publicKey lengths do not match', (): void => { + expect( + () => verify( + new Uint8Array([0x61, 0x62, 0x63, 0x64]), + new Uint8Array(64), + new Uint8Array(31) + ) + ).toThrow(/Invalid publicKey/); + }); + + it('throws error when signature lengths do not match', (): void => { + expect( + () => verify( + new Uint8Array([0x61, 0x62, 0x63, 0x64]), + new Uint8Array(65), + new Uint8Array(32) + ) + ).toThrow(/Invalid signature/); + }); }); diff --git a/packages/util-crypto/src/schnorrkel/verify.ts b/packages/util-crypto/src/schnorrkel/verify.ts index e60a6ed46..72d234a1a 100644 --- a/packages/util-crypto/src/schnorrkel/verify.ts +++ b/packages/util-crypto/src/schnorrkel/verify.ts @@ -4,7 +4,7 @@ import '../polyfill'; -import { u8aToU8a } from '@polkadot/util'; +import { assert, u8aToU8a } from '@polkadot/util'; import { sr25519Verify } from '@polkadot/wasm-crypto'; /** @@ -13,8 +13,11 @@ import { sr25519Verify } from '@polkadot/wasm-crypto'; */ export default function schnorrkelVerify (message: Uint8Array | string, signature: Uint8Array | string, publicKey: Uint8Array | string): boolean { const messageU8a = u8aToU8a(message); - const signatureU8a = u8aToU8a(signature); const publicKeyU8a = u8aToU8a(publicKey); + const signatureU8a = u8aToU8a(signature); + + assert(publicKeyU8a.length === 32, `Invalid publicKey, received ${publicKeyU8a.length} bytes, expected 32`); + assert(signatureU8a.length === 64, `Invalid signature, received ${signatureU8a.length} bytes, expected 64`); return sr25519Verify(signatureU8a, messageU8a, publicKeyU8a); } diff --git a/packages/util-crypto/src/signature/verify.ts b/packages/util-crypto/src/signature/verify.ts index b2053ea44..e6f321af0 100644 --- a/packages/util-crypto/src/signature/verify.ts +++ b/packages/util-crypto/src/signature/verify.ts @@ -67,10 +67,9 @@ export default function signatureVerify (message: Uint8Array | string, signature const result: VerifyResult = { crypto: 'none', isValid: false }; const publicKey = addressDecode(addressOrPublicKey); + const isMultisig = [0, 1, 2].includes(signatureU8a[0]) && [65, 66].includes(signatureU8a.length); - const multisig = [0, 1, 2].includes(signatureU8a[0]) && [65, 66].includes(signatureU8a.length); - - if (multisig) { + if (isMultisig) { return verifyMultisig(result, message, signatureU8a, publicKey); } else { return verifyDetect(result, message, signatureU8a, publicKey);