BadgerDAO ibBTC Wrapper contest - pauliax's results

Handle

pauliax

Vulnerability details

Impact

There are many external risks (mentioned https://github.com/code-423n4/2021-10-badgerdao#risks) so my suggestion is that you should consider making the contracts pausable, so in case of an unexpected event, the governance can pause transfers.

Recommended Mitigation Steps

Consider making contracts Pausable https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/security/Pausable.sol

Handle

pauliax

Vulnerability details

Impact

'immutable' greatly reduces gas costs. There is a variable that does not change so it can be marked as immutable to improve the gas costs: ERC20Upgradeable public ibbtc;

Handle

pauliax

Vulnerability details

Impact

You import ICore interface but actually need only one function from it: pricePerShare(). Consider importing a minimal ICore interface with only the functions that you actually use to reduce deployment gas costs. Or you can just simply re-use ICoreOracle.

Handle

pauliax

Vulnerability details

Impact

modifier onlyOracle in WrappedIbbtc is never used, so can be removed to reduce deployment gas costs.

Handle

pauliax

Vulnerability details

Impact

functions mint, burn, transfer and transferFrom could skip other steps if the amount is 0.

Handle

pauliax

Vulnerability details

Impact

function balanceToShares should handle a case when pricePerShare drops to 0. Currently, this will produce a runtime division by zero error. While in theory, this is an unlikely scenario but in practice, you should consider returning 0 in such a case.

Recommended Mitigation Steps

Return 0 if pricePerShare is 0.

Handle

pauliax

Vulnerability details

Impact

function updatePricePerShare declares to return something but actually does not return anything: function updatePricePerShare() public virtual returns (uint256)

This may confuse callers.

Recommended Mitigation Steps

Probably the intention was to return pricePerShare.