Merge pull request #2929 from chirag-parmar/2.x

Length check and formatting private key in accounts.privateKeyToAccount method

Author: nivida

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)

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);
     }
 

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,19 +233,17 @@ 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);
 
         pbkdf2Sync.mockReturnValueOnce(Buffer.from('00000000000000000000000000000000'));
 
         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);