Platform: Code4rena
Start Date: 14/11/2021
Pot Size: $30,000 USDC
Total HM: 7
Participants: 13
Period: 3 days
Judge: leastwood
Total Solo HM: 4
Id: 57
League: ETH
Rank: 2/13
Findings: 5
Award: $6,483.34
🌟 Selected for report: 5
🚀 Solo Findings: 1
🌟 Selected for report: WatchPug
Also found by: 0x0x0x, Meta0xNull, fatima_naz, gzeon, ksk2345
gzeon
setGuardian incorrectly set governance in SettToRenIbbtcZap.sol
_onlyGovernance(); governance = _guardian; }``` ## Tools Used ## Recommended Mitigation Steps
#0 - GalloDaSballo
2021-11-17T15:09:50Z
Agree, there' s a bunch of duplicates for this, I think #31 #10 and a few more I don't remember
#1 - CloudEllie
2021-11-17T17:35:59Z
Also #25 (potentially), based on the comments I've seen
#2 - 0xleastwood
2021-12-09T12:15:25Z
duplicate of #51
gzeon
redeem may return less than minOut if wrong token is supplied with poolID=3 because there is no check against minOut in L230-L232.
After L230
require(token==byvWBTC, "INVALID_TOKEN");
or
After L236
require(out>=minOut, "INSUFFICIENT_OUTPUT");
#0 - GalloDaSballo
2021-11-17T15:11:32Z
Agree with the finding, we will put a slippage check at end of function
Seems to be a dup of #47
#1 - 0xleastwood
2021-12-09T13:03:19Z
duplicate of #47
gzeon
There is no slippage control on deposit of IbbtcVaultZap.sol, which expose user to sandwich attack.
https://github.com/Badger-Finance/badger-ibbtc-utility-zaps/blob/6f700995129182fec81b772f97abab9977b46026/contracts/IbbtcVaultZap.sol#L174 Any deposit can be sandwiched, especially when the pool is not balanced.
Add a _minOut in line with the mint function of other contacts, and pass it as a parameter on L174
🌟 Selected for report: gzeon
gzeon
calcMint in Zap.sol always return poolId=0 and idx=0, while the docstring specified it should return the most optimal route instead. This will lead to suboptimal zap.
#0 - GalloDaSballo
2021-11-17T15:15:48Z
Given the context that the warden has, the finding is valid, we're missing two functions for calcMint
As for us, we have shifted to only using pool 0 as such the code works fine for us
gzeon
Use else if for mutually exclusive conditions to save gas
https://github.com/Badger-Finance/ibbtc/blob/d8b95e8d145eb196ba20033267a9ba43a17be02c/contracts/Zap.sol#L125 can be rewritten into
function _addLiquidity(ICurveFi pool, uint amount, uint numTokens, uint idx) internal { if (numTokens == 2) { uint[2] memory amounts; amounts[idx] = amount; pool.add_liquidity(amounts, 0); } else if (numTokens == 3) { uint[3] memory amounts; amounts[idx] = amount; pool.add_liquidity(amounts, 0); } else if (numTokens == 4) { uint[4] memory amounts; amounts[idx] = amount; pool.add_liquidity(amounts, 0); } }
#0 - GalloDaSballo
2021-11-17T15:33:45Z
Would like to see proof to this statement, why does it save gas?
#1 - 0xleastwood
2021-12-09T11:24:22Z
I imagine gas savings could be generated on this one by limiting the number of comparisons the function makes. If an earlier branch is entered, subsequent branches are not executed and the function will instead return.
🌟 Selected for report: gzeon
gzeon
Detailed description of the impact of this finding.
L111 and L234 are unreachable because pools is a Pool[4], any poolId > 3 would have been reverted in L102 and L225 respectively. We can even replace else if (poolId == 3) to else to save a little bit more gas.
#0 - GalloDaSballo
2021-11-17T15:30:47Z
Agree with the finding but probably nofix as in the future we may add more pools
🌟 Selected for report: gzeon
gzeon
https://github.com/Badger-Finance/badger-ibbtc-utility-zaps/blob/6f700995129182fec81b772f97abab9977b46026/contracts/IbbtcVaultZap.sol#L158
depositAmounts[i] += _amounts[i];
can be
depositAmounts[i] = _amounts[i];
instead
#0 - GalloDaSballo
2021-11-17T15:29:32Z
Agree with the finding, we can just use =