BadgerDAO ibBTC Wrapper contest - loop's results

Handle

loop

Vulnerability details

The price of pricePerShare in WrappedIbbtcEth.sol is dependant on two things:

• The pricePerShare of core.
• updatePricePerShare being invoked every X time to update the pricePerShare of wibBTC.

The only time updatePricePerShare is invoked inside the contract is during the initialize function, all other instances have to be an external call. When invoking updatePricePerShare the lastPricePerShareUpdate value is set to the current time, which is likely to insure the price stays up-to-date. Currently the contract does not check whether the price is up-to-date or whether it has been updated recently. Meanwhile, balanceOf, totalSupply, balanceToShares and sharesToBalance depend on pricePerShare being up-to-date. While it's not a direct issue for these functions specifically considering they are restricted to view, both transfer and transferFrom use balanceToShares and are thus indirectly dependent on pricePerShare. Since value is being transferred in these functions it would be good to ensure the pricePerShare is up-to-date before completing the transfer.

Impact

If I understand it correctly, an arbitrage like this might be possible:

• Track the pricePerShare for both Ibbtc and wIbbtc.
• If the pricePerShare for Ibbtc is lower than wIbbtc, mint wIbbtc using Ibbtc as collateral.
• Swap minted wIbbtc for sbtc on Curve.
• Invoke updatePricePerShare in WrappedIbbtcEth to update lastPricePerShare to the new lower value.
• Swap sbtc back to wIbbtc, which will result in more wIbbtc than before due to the transfer function now calculating a higher amount of shares using balanceToShares.

I may be wrong here due to limited understanding of Curve pools, but I reckon either transfer or transferFrom will be used to swap wIbbtc, both of which use balanceToShares to calculate the amount that will actually be sent.

Proof of Concept

updatePricePerShare: https://github.com/code-423n4/2021-10-badgerdao/blob/main/contracts/WrappedIbbtcEth.sol#L72-L77

transfer/transferFrom relevant parts:

• https://github.com/code-423n4/2021-10-badgerdao/blob/main/contracts/WrappedIbbtcEth.sol#L108-L110
• https://github.com/code-423n4/2021-10-badgerdao/blob/main/contracts/WrappedIbbtcEth.sol#L127-L129

balanceToShares: https://github.com/code-423n4/2021-10-badgerdao/blob/main/contracts/WrappedIbbtcEth.sol#L155-L157

Recommended Mitigation Steps

Add a require statement for transfer and transferFrom to ensure pricePerShare is updated either using something like require(block.timestamp > lastPricePerShare + x) with x as the chosen time interval or require(pricePerShare == core.pricePerShare()).