diff --git a/CHANGELOG.md b/CHANGELOG.md index a8023aee21d..95419d54a63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [2.0.0-alpha.1] +### Added + +- Length check of the PK added to the ``fromPrivateKey`` method of the ``Account`` model (#2928) + ### Fixed - miner.startMining fixed (#2877) diff --git a/packages/web3-eth-accounts/src/models/Account.js b/packages/web3-eth-accounts/src/models/Account.js index 2c0ff056e6e..60279d17fd5 100644 --- a/packages/web3-eth-accounts/src/models/Account.js +++ b/packages/web3-eth-accounts/src/models/Account.js @@ -123,6 +123,15 @@ export default class Account { * @returns {Account} */ static fromPrivateKey(privateKey, accounts = {}) { + if (!privateKey.startsWith('0x')) { + privateKey = '0x' + privateKey; + } + + // 64 hex characters + hex-prefix + if (privateKey.length !== 66) { + throw new Error("Private key must be 32 bytes long"); + } + return new Account(EthLibAccount.fromPrivate(privateKey), accounts); } diff --git a/packages/web3-eth-accounts/tests/src/models/AccountTest.js b/packages/web3-eth-accounts/tests/src/models/AccountTest.js index 37c1b9bac84..92a850c522b 100644 --- a/packages/web3-eth-accounts/tests/src/models/AccountTest.js +++ b/packages/web3-eth-accounts/tests/src/models/AccountTest.js @@ -25,9 +25,10 @@ jest.mock('../../../src/Accounts'); * AccountTest test */ describe('AccountTest', () => { - let account, accountsMock, transactionSignerMock; + let account, accountsMock, transactionSignerMock, mockKey; beforeEach(() => { + mockKey = '0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef'; transactionSignerMock = new TransactionSigner(); new Accounts(); @@ -55,6 +56,20 @@ describe('AccountTest', () => { expect(accountsMock.signTransaction).toHaveBeenCalledWith({}, 'pk', callback); }); + it('calls fromPrivateKey with incorrect private key length and throws error', () => { + expect(() => { + Account.fromPrivateKey('asdfasdf') + }).toThrow('Private key must be 32 bytes long'); + }); + + it('calls fromPrivateKey with incorrect private key prefix and throws error', () => { + mockKey = '0z0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef'; + + expect(() => { + Account.fromPrivateKey(mockKey) + }).toThrow('Private key must be 32 bytes long'); + }); + it('calls sign with non-strict hex and returns the expected string', () => { isHexStrict.mockReturnValue(false); @@ -152,17 +167,15 @@ describe('AccountTest', () => { final: jest.fn() }; - decipher.update.mockReturnValueOnce(Buffer.from('0')); + decipher.update.mockReturnValueOnce(Buffer.from(mockKey.slice(2,34), 'hex')); - decipher.final.mockReturnValueOnce(Buffer.from('0')); + decipher.final.mockReturnValueOnce(Buffer.from(mockKey.slice(34,66), 'hex')); createDecipheriv.mockReturnValueOnce(decipher); expect(Account.fromV3Keystore(json, 'password', false)).toBeInstanceOf(Account); - expect(fromPrivate).toHaveBeenLastCalledWith( - `0x${Buffer.concat([Buffer.from('0'), Buffer.from('0')]).toString('hex')}` - ); + expect(fromPrivate).toHaveBeenLastCalledWith(mockKey); expect(scrypt).toHaveBeenCalledWith( Buffer.from('password'), @@ -220,9 +233,9 @@ describe('AccountTest', () => { final: jest.fn() }; - decipher.update.mockReturnValueOnce(Buffer.from('0')); + decipher.update.mockReturnValueOnce(Buffer.from(mockKey.slice(2,34), 'hex')); - decipher.final.mockReturnValueOnce(Buffer.from('0')); + decipher.final.mockReturnValueOnce(Buffer.from(mockKey.slice(34,66), 'hex')); createDecipheriv.mockReturnValueOnce(decipher); @@ -230,9 +243,7 @@ describe('AccountTest', () => { expect(Account.fromV3Keystore(json, 'password', false)).toBeInstanceOf(Account); - expect(fromPrivate).toHaveBeenCalledWith( - `0x${Buffer.concat([Buffer.from('0'), Buffer.from('0')]).toString('hex')}` - ); + expect(fromPrivate).toHaveBeenCalledWith(mockKey); expect(pbkdf2Sync).toHaveBeenCalledWith( Buffer.from('password'), @@ -349,7 +360,7 @@ describe('AccountTest', () => { uuid.v4.mockReturnValueOnce(0); - expect(Account.fromPrivateKey('pk').toV3Keystore('password', options)).toEqual({ + expect(Account.fromPrivateKey(mockKey).toV3Keystore('password', options)).toEqual({ version: 3, id: 0, address: 'a', @@ -369,7 +380,7 @@ describe('AccountTest', () => { } }); - expect(fromPrivate).toHaveBeenCalledWith('pk'); + expect(fromPrivate).toHaveBeenCalledWith(mockKey); expect(randomBytes).toHaveBeenNthCalledWith(1, 32); @@ -426,7 +437,7 @@ describe('AccountTest', () => { uuid.v4.mockReturnValueOnce(0); - expect(Account.fromPrivateKey('pk').toV3Keystore('password', options)).toEqual({ + expect(Account.fromPrivateKey(mockKey).toV3Keystore('password', options)).toEqual({ version: 3, id: 0, address: 'a', @@ -445,7 +456,7 @@ describe('AccountTest', () => { } }); - expect(fromPrivate).toHaveBeenCalledWith('pk'); + expect(fromPrivate).toHaveBeenCalledWith(mockKey); expect(randomBytes).toHaveBeenNthCalledWith(1, 32); @@ -484,10 +495,10 @@ describe('AccountTest', () => { randomBytes.mockReturnValue(Buffer.from('random')); expect(() => { - Account.fromPrivateKey('pk').toV3Keystore('password', {kdf: 'nope'}); + Account.fromPrivateKey(mockKey).toV3Keystore('password', {kdf: 'nope'}); }).toThrow('Unsupported kdf'); - expect(fromPrivate).toHaveBeenCalledWith('pk'); + expect(fromPrivate).toHaveBeenCalledWith(mockKey); expect(randomBytes).toHaveBeenNthCalledWith(1, 32); @@ -511,10 +522,10 @@ describe('AccountTest', () => { keccak256.mockReturnValueOnce('0xmac'); expect(() => { - Account.fromPrivateKey('pk').toV3Keystore('password', options); + Account.fromPrivateKey(mockKey).toV3Keystore('password', options); }).toThrow('Unsupported cipher'); - expect(fromPrivate).toHaveBeenCalledWith('pk'); + expect(fromPrivate).toHaveBeenCalledWith(mockKey); expect(randomBytes).toHaveBeenNthCalledWith(1, 32);