Platform: Code4rena
Start Date: 27/05/2022
Pot Size: $75,000 USDC
Total HM: 20
Participants: 58
Period: 7 days
Judge: GalloDaSballo
Total Solo HM: 15
Id: 131
League: ETH
Rank: 44/58
Findings: 1
Award: $159.01
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x29A, 0xNazgul, 0xf15ers, BowTiedWardens, Chom, Funen, Kaiziron, Kumpa, MiloTruck, Picodes, Ruhum, SecureZeroX, Sm4rty, SmartSek, StyxRave, WatchPug, Waze, asutorufos, bardamu, berndartmueller, c3phas, catchup, cccz, codexploder, cryptphi, defsec, delfin454000, dipp, fatherOfBlocks, gzeon, hake, hansfriese, hyh, masterchief, oyc_109, sach1r0, sashik_eth, shenwilly, simon135, unforgiven
159.0051 USDC - $159.01
AddressProvider.sol
does not have a getSwapperRouter
functionIn FeeBurner.sol
, the _swapperRouter
function gets the swapper router from the address provider. The AddressProvider.sol
and IAddressProvider.sol
contracts do not seem to have a getSwapperRouter
function. If the _addressProvider
in FeeBurner.sol
is the AddressProvider.sol
contract or implements IAddressProvider.sol
then FeeBurner.sol
would not function correctly or at all.
The _swapperRouter function is called in the burnToTarget
function which should then revert thus making burnToTarget
uncallable.
Add the getSwapperRouter
function in the AddressProvider.sol
and IAddressProvider.sol
contracts.
FeeBurner.sol
In the burnToTarget
function of FeeBurner.sol
, if the tokens being transferred are tokens whose balances change during a transfer such as deflationary and fee-on-transfer tokens, then the swap could fail since the swap input amount might be larger than the actual amount of tokens received in FeeBurner.sol
.
When tokens are transferred to the contract, check the balance of the contract before and after the transfer. Use the difference in the balances as the input amount for the swap call.
mint
is not checked in InflationManager.sol
In Minter.sol
, if inflation has not started and lastEvent == 0
then the mint
function will return false. In InflationManager.sol
, the mintRewards
function does not check the return value for the mint call. When mintRewards
is called by a gauge and fails, the transaction does not revert.
When calling claimRewards
in AmmGauge.sol
, the call to mintRewards
does not revert if minting has failed and the user loses their rewards.
function claimRewards(address beneficiary) external virtual override returns (uint256) { require( msg.sender == beneficiary || _roleManager().hasRole(Roles.GAUGE_ZAP, msg.sender), Error.UNAUTHORIZED_ACCESS ); _userCheckpoint(beneficiary); uint256 amount = perUserShare[beneficiary]; if (amount <= 0) return 0; perUserShare[beneficiary] = 0; controller.inflationManager().mintRewards(beneficiary, amount); emit RewardClaimed(beneficiary, amount); return amount; }
The KeeperGauge.sol
and LpGauge.sol
contracts would fail in a similar way in addition to any other gauges that call mintRewards
.
Consider either returning a bool value for the mintRewards
function in InflationManager.sol
and check the return value in the gauges or insert the mint call in a require,
require(Minter(minter).mint(beneficiary, amount))
#0 - GalloDaSballo
2022-06-21T00:18:54Z
Great find!!
##Â 2. Incompatibility with deflationary/rebase/fee-on-transfer tokens in FeeBurner.sol Agree as non-critical
Great find!
The findings in this report are very unique, good job!
#1 - GalloDaSballo
2022-06-23T17:40:34Z
Actually after re-review finding 3 is not valid as _mint
always returns true or reverts