Skip to content

Commit

Permalink
fix(security-issues): remove web3 and @metamask/eth-sig-util deps, re…
Browse files Browse the repository at this point in the history 
…placed with ethers
  • Loading branch information
@krboktv
krboktv committed Jul 4, 2024
1 parent dc6c483 commit fde3bc3
Show file tree
Hide file tree
Showing 19 changed files with 213 additions and 1,902 deletions.
14 changes: 10 additions & 4 deletions .eslintrc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,18 @@ module.exports = {
'no-unused-vars': 'off',
'max-len': ['error', {code: 100}],
'max-depth': ['error', 3],
'max-lines-per-function': ['error', 32],
'max-lines-per-function': ['error', 40],
'max-params': ['error', 6],
'@typescript-eslint/no-explicit-any': 'error',
'@typescript-eslint/no-unused-vars': 'error',
'unused-imports/no-unused-imports': 'error',
'unused-imports/no-unused-vars': 0,
'@typescript-eslint/no-unused-vars': [
"warn",
{
"argsIgnorePattern": "^_",
"varsIgnorePattern": "^_",
"caughtErrorsIgnorePattern": "^_"
}
],
'unused-imports/no-unused-imports': 'error'
},
overrides: [
{
Expand Down
7 changes: 2 additions & 5 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,8 @@
},
"homepage": "https://github.com/1inch/permit-signed-approvals-utils#readme",
"dependencies": {
"@metamask/eth-sig-util": "7.0.0",
"@uniswap/permit2-sdk": "^1.2.0",
"i": "^0.3.7",
"web3": "^1.5.1",
"web3-utils": "^1.5.1"
"ethers": "^6.13.1"
},
"devDependencies": {
"@1inch/solidity-utils": "3.5.5",
Expand All @@ -46,6 +43,7 @@
"@openzeppelin/contracts": "5.0.1",
"@types/jest": "29.5.11",
"@types/node": "20.11.4",
"@types/ws": "^8.5.10",
"@typescript-eslint/eslint-plugin": "6.19.0",
"babel-jest": "29.7.0",
"chai": "4.3.7",
Expand All @@ -56,7 +54,6 @@
"eslint-plugin-node": "11.1.0",
"eslint-plugin-promise": "4.3.1",
"eslint-plugin-unused-imports": "3.0.0",
"ethers": "6.9.0",
"hardhat": "2.19.4",
"hardhat-dependency-compiler": "1.1.3",
"hardhat-deploy": "0.11.45",
Expand Down
64 changes: 0 additions & 64 deletions src/__snapshots__/eip-2612-permit.helper.test.ts.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,6 @@ exports[`eip-2612 permit helpers buildPermitTypedData() 1`] = `
},
"primaryType": "Permit",
"types": {
"EIP712Domain": [
{
"name": "name",
"type": "string",
},
{
"name": "version",
"type": "string",
},
{
"name": "chainId",
"type": "uint256",
},
{
"name": "verifyingContract",
"type": "address",
},
],
"Permit": [
{
"name": "owner",
Expand Down Expand Up @@ -77,20 +59,6 @@ exports[`eip-2612 permit helpers buildPermitTypedData({isDomainWithoutVersion: t
},
"primaryType": "Permit",
"types": {
"EIP712Domain": [
{
"name": "name",
"type": "string",
},
{
"name": "chainId",
"type": "uint256",
},
{
"name": "verifyingContract",
"type": "address",
},
],
"Permit": [
{
"name": "owner",
Expand Down Expand Up @@ -133,20 +101,6 @@ exports[`eip-2612 permit helpers buildPermitTypedData({isSaltInsteadOfChainId: t
},
"primaryType": "Permit",
"types": {
"EIP712Domain": [
{
"name": "name",
"type": "string",
},
{
"name": "verifyingContract",
"type": "address",
},
{
"name": "salt",
"type": "bytes32",
},
],
"Permit": [
{
"name": "owner",
Expand Down Expand Up @@ -190,24 +144,6 @@ exports[`eip-2612 permit helpers buildPermitTypedData({isSaltInsteadOfChainId: t
},
"primaryType": "Permit",
"types": {
"EIP712Domain": [
{
"name": "name",
"type": "string",
},
{
"name": "version",
"type": "string",
},
{
"name": "verifyingContract",
"type": "address",
},
{
"name": "salt",
"type": "bytes32",
},
],
"Permit": [
{
"name": "owner",
Expand Down
3 changes: 3 additions & 0 deletions src/connector/abi-coder.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import {AbiCoder} from 'ethers';

export const abiCoder = new AbiCoder()
32 changes: 4 additions & 28 deletions src/connector/private-key-provider.connector.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import Web3 from 'web3';
import {instance, mock, verify, when} from 'ts-mockito';
import {Eth} from 'web3-eth';
import {instance, mock} from 'ts-mockito';
import {PrivateKeyProviderConnector} from './private-key-provider.connector';
import {buildPermitTypedData} from '../eip-2612-permit.helper';
import {EIP_2612_PERMIT_ABI} from '../eip-2612-permit.const';
import {PermitParams} from '../model/permit.model';
import {Web3Like} from './web3';

describe('PrivateKeyProviderConnector', () => {
let web3Provider: Web3;
let web3Provider: Web3Like;
let privateKeyProviderConnector: PrivateKeyProviderConnector;

const testPrivateKey =
Expand All @@ -33,7 +31,7 @@ describe('PrivateKeyProviderConnector', () => {
});

beforeEach(() => {
web3Provider = mock<Web3>();
web3Provider = mock<Web3Like>();
privateKeyProviderConnector = new PrivateKeyProviderConnector(
testPrivateKey,
instance(web3Provider)
Expand All @@ -55,26 +53,4 @@ describe('PrivateKeyProviderConnector', () => {

expect(signature).toBe(expectedSignature);
});

it('contractEncodeABI() changed address from null to undefined for contract instance', async () => {
const eth = mock<Eth>();
class ContractMock {
methods = {
foo: () => ({encodeABI: () => ''}),
};
}

/* eslint-disable-next-line @typescript-eslint/no-explicit-any */
when(eth.Contract).thenReturn(ContractMock as any);
when(web3Provider.eth).thenReturn(instance(eth));

privateKeyProviderConnector.contractEncodeABI(
EIP_2612_PERMIT_ABI,
null,
'foo',
[]
);

verify(eth.Contract).once();
});
});
47 changes: 25 additions & 22 deletions src/connector/private-key-provider.connector.ts
View file
Original file line number Diff line number Diff line change
@@ -1,41 +1,43 @@
import {ProviderConnector} from './provider.connector';
import Web3 from 'web3';
import {EIP712TypedData} from '../model/eip712.model';
import {AbiItem} from '../model/abi.model';
import {AbiInput, AbiItem as Web3AbiItem} from 'web3-utils';
import {signTypedData, SignTypedDataVersion} from '@metamask/eth-sig-util';
import {AbiInput, AbiItem} from '../model/abi.model';
import {Interface, ParamType, Wallet, InterfaceAbi} from 'ethers'
import {add0x} from '../helpers/add-0x';
import {TypedDataDomain} from 'ethers/src.ts/hash';
import {abiCoder} from './abi-coder';
import {Web3Like} from './web3';

export class PrivateKeyProviderConnector implements ProviderConnector {

private readonly wallet: Wallet

constructor(
private readonly privateKey: string,
protected readonly web3Provider: Web3
) {}
readonly privateKey: string,
protected readonly web3Provider: Web3Like
) {
this.wallet = new Wallet(add0x(privateKey))
}

contractEncodeABI(
abi: AbiItem[],
address: string | null,
_address: string | null,
methodName: string,
methodParams: unknown[]
): string {
const contract = new this.web3Provider.eth.Contract(
abi as Web3AbiItem[],
address === null ? undefined : address
);

return contract.methods[methodName](...methodParams).encodeABI();
const contract = new Interface(abi as InterfaceAbi)
return contract.encodeFunctionData(methodName, methodParams).toString()
}

signTypedData(
_walletAddress: string,
typedData: EIP712TypedData,
/* eslint-disable-next-line @typescript-eslint/no-unused-vars */
_typedDataHash = ''
): Promise<string> {
const result = signTypedData({
privateKey: Buffer.from(this.privateKey, 'hex'),
data: typedData,
version: SignTypedDataVersion.V4,
});
const result = this.wallet.signTypedData(
typedData.domain as TypedDataDomain,
typedData.types,
typedData.message
)

return Promise.resolve(result);
}
Expand All @@ -48,10 +50,11 @@ export class PrivateKeyProviderConnector implements ProviderConnector {
}

decodeABIParameter<T>(type: string, hex: string): T {
return this.web3Provider.eth.abi.decodeParameter(type, hex) as T;
return abiCoder.decode([type], hex)[0] as T;
}

decodeABIParameters<T>(types: AbiInput[], hex: string): T {
return this.web3Provider.eth.abi.decodeParameters(types, hex) as T;
const formattedTypes: ReadonlyArray<ParamType> = types.map((type) => ParamType.from(type))
return abiCoder.decode(formattedTypes, hex) as T;
}
}
3 changes: 1 addition & 2 deletions src/connector/provider.connector.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {EIP712TypedData} from '../model/eip712.model';
import {AbiItem} from '../model/abi.model';
import {AbiInput} from 'web3-utils';
import {AbiInput, AbiItem} from '../model/abi.model';

export interface ProviderConnector {
contractEncodeABI(
Expand Down
53 changes: 7 additions & 46 deletions src/connector/web3-provider.connector.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import {Web3ProviderConnector} from './web3-provider.connector';
import Web3 from 'web3';
import { anything, instance, mock, verify, when } from 'ts-mockito';
import {Eth} from 'web3-eth';
import { anything, instance, mock, when } from 'ts-mockito';
import {buildPermitTypedData} from '../eip-2612-permit.helper';
import {EIP_2612_PERMIT_ABI} from '../eip-2612-permit.const';
import {Web3Like} from './web3';

describe('Web3ProviderConnector', () => {
let web3Provider: Web3;
let web3Provider: Web3Like;
let web3ProviderConnector: Web3ProviderConnector;

const tokenAddress = '0x111111111117dc0aa78b770fa6a738034120c302';
Expand All @@ -20,7 +18,7 @@ describe('Web3ProviderConnector', () => {
);

beforeEach(() => {
web3Provider = mock<Web3>();
web3Provider = mock<Web3Like>();
web3ProviderConnector = new Web3ProviderConnector(
instance(web3Provider)
);
Expand All @@ -40,28 +38,6 @@ describe('Web3ProviderConnector', () => {
expect(extendedWeb3.signTypedDataV4).toHaveBeenCalledWith(walletAddress, JSON.stringify(typedData));
});

it('contractEncodeABI() changed address from null to undefined for contract instance', async () => {
const eth = mock<Eth>();
class ContractMock {
methods = {
foo: () => ({encodeABI: () => ''}),
};
}

/* eslint-disable-next-line @typescript-eslint/no-explicit-any */
when(eth.Contract).thenReturn(ContractMock as any);
when(web3Provider.eth).thenReturn(instance(eth));

web3ProviderConnector.contractEncodeABI(
EIP_2612_PERMIT_ABI,
null,
'foo',
[]
);

verify(eth.Contract).once();
});

it('ethCall() should create a contact call', async () => {
const ethCall = jest.fn();
const expectedResult = 'test';
Expand Down Expand Up @@ -90,26 +66,11 @@ describe('Web3ProviderConnector', () => {
});

it('decodeABIParameter()', () => {
const decodeParameter = jest.fn();
const expectedResult = 'test';
const type = 'type';
const hex = 'hex';

when(web3Provider.eth).thenReturn({
abi: {
decodeParameter
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any);
decodeParameter.mockReturnValue(expectedResult);

const result = web3ProviderConnector.decodeABIParameter(
type,
hex
'uint256',
'0x0000000000000000000000000000000000000000000000000000000000000110'
);

expect(result).toBe(expectedResult);
expect(decodeParameter).toHaveBeenCalledTimes(1);
expect(decodeParameter).toHaveBeenCalledWith(type, hex);
expect(result).toBe(272n)
});
});
Loading

0 comments on commit fde3bc3

Please sign in to comment.