Fujicracy / fuji-v2

Cross-chain money market aggregator
https://fuji-v2-frontend.vercel.app
15 stars 10 forks source link

Incorrect overloading of _spendAllowance #367

Closed pedrovalido closed 1 year ago

pedrovalido commented 1 year ago

Description

BaseVault.sol intends to overload _spendAllowance so that it takes withdrawal allowance into consideration for transferFrom calls.

However, defined function signature of spendAllowance in BaseVault :

_spendAllowance(address, address, address, uint256) here

https://github.com/Fujicracy/fuji-v2/blob/50fd0b74ccee1a73a459118e50e044a2bcfacd10/packages/protocol/src/abstracts/BaseVault.sol#L197

is different from the function signature ERC20 expects or calls:

_spendAllowance(address, address, uint256) here

https://github.com/Fujicracy/fuji-v2/blob/50fd0b74ccee1a73a459118e50e044a2bcfacd10/packages/protocol/src/abstracts/BaseVault.sol#L197

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/5e76b2622546a2e42e5c19e4ce1a96ee2691c7fe/contracts/token/ERC20/ERC20.sol#L324

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/5e76b2622546a2e42e5c19e4ce1a96ee2691c7fe/contracts/token/ERC20/ERC20.sol#L324

Therefore, if someone executes a transferFrom, it will call the vanilla ERC20's spendAllowance, without taking withdrawal allowances into consideration. This allows owners to transfer shares to a new address using transferFrom, while still retaining their withdrawal balance.

Remediation

Consider overloading spendAllowance with the right function signature.