api3dao / api3-dao-dashboard

API3 DAO dashboard
api3.eth/
14 stars 14 forks source link

Function signature doesn't get verified #263

Open bbenligiray opened 2 years ago

bbenligiray commented 2 years ago

The dashboard doesn't verify function signatures and allows proposals with invalid function signatures to be created (https://api3.eth.link/#/history/secondary-13)

There is no utility function for verifying signatures in ethers v5, but there is a workaround, see https://github.com/ethers-io/ethers.js/issues/1101 (Edit: we already implement the workaround)

Siegrift commented 2 years ago

Some validation is done by https://github.com/api3dao/api3-dao-dashboard/blob/main/src/logic/proposals/encoding/encoding.ts#L80 and it does catch some bugs (e.g. unit instead of uint https://github.com/api3dao/api3-dao-dashboard/blob/main/cypress/integration/new-proposal.spec.ts#L29)

unfortunately it doesn't validate spaces properly...

bbenligiray commented 2 years ago

How does that work? When I run ethers.utils.FunctionFragment.from("transfer(address, unit256)") on https://playground.ethers.org/ it doesn't throw.

Siegrift commented 2 years ago

You're right - that catches only basic validation errors like '' (empty string) value. More validation is performed by encoding the parameters which catches the unit256 mistake - but that does not involve the function fragment anymore...

bbenligiray commented 2 years ago

You're right, and that's essentially the workaround they propose in the ethers.js issue. I'm still keeping this issue open in case a function signature verification function gets implemented in ethers v6 that can catch things like (address,uint256)transfer (though I doubt it)