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: 3/13
Findings: 2
Award: $3,761.77
🌟 Selected for report: 2
🚀 Solo Findings: 1
🌟 Selected for report: Ruhum
Ruhum
In the redeem() function, the user can pass a token address. That's the token they receive in return for the ibbtc they give back. Because of missing address checks the user can provide any possible ERC20 token here without the function reverting.
Although it's not strictly specified in the code I expect that the user should only be able to redeem wBTC or renBTC tokens since they should also only be able to deposit those.
Manual Analysis
Verify that the passed token address is either wBTC or renbtc
#0 - tabshaikh
2021-11-14T19:41:04Z
best practice to add wBTC or renbtc in require, disagree on the severity
#1 - GalloDaSballo
2021-11-17T14:57:41Z
Agree with the finding since only user can rekt themselves I believe this to be a medium severity finding we'll mitigate by adding a slippage check at the end of the function
🌟 Selected for report: Ruhum
940.442 USDC - $940.44
Ruhum
The user can call the mint() function with any contract address that implements the safeTransferFrom() function. Thus, they can mint as many ibbtc tokens for free as they want.
Technically, they can deploy a contract that has a safeTransferFrom() function that simply returns true. They then call the mint() function and pass the contract address and an arbitrary value for the amount parameter. The mint() function will then go ahead and mint them the passed amount of ibbtc tokens without receiving anything in return.
There are no checks that verify that the passed token address is either the wBTC or renBTC token.
Manual Analysis
Add the following check to the beginning of the function:
require(token == address(ren) || token == address(wbtc);
#0 - tabshaikh
2021-11-14T09:18:58Z
We should totally add the require(token == address(ren) || token == address(wbtc); as a best practice. Disagree on the risk as users would not be able to mint ibbtc for free as firstly the contract is to hold no token and this will revert https://github.com/Badger-Finance/ibbtc/blob/d8b95e8d145eb196ba20033267a9ba43a17be02c/contracts/Zap.sol#L129 if ren/ wbtc amount is not present
#1 - GalloDaSballo
2021-11-17T14:45:16Z
Disagree with the finding, the finding claims that a user would be able to mint ibBTC with any token, this is not correct.
Using any token beside the supported ones, would cause a revert when adding liquidity to the pool or when we deposit in the yearn vault.
See code for revert: https://github.com/Badger-Finance/ibbtc/blob/d8b95e8d145eb196ba20033267a9ba43a17be02c/contracts/Zap.sol#L104 https://github.com/Badger-Finance/ibbtc/blob/d8b95e8d145eb196ba20033267a9ba43a17be02c/contracts/Zap.sol#L108
#2 - 0xleastwood
2021-12-09T12:58:18Z
agree with sponsor, user's tx will revert if _addLiquidity or deposit is called. The two functions will attempt to pull the correct amount of funds from Zap.sol which would be 0 as in the warden's suggested attack vector.
#3 - 0xleastwood
2021-12-09T12:58:58Z
Will keep as low risk as it is useful to check token is either address(ren) or address(wbtc).