BadgerDAO ibBTC Wrapper contest - defsec's results

Handle

defsec

Vulnerability details

Impact

The initialize function that initializes important contract state can be called by anyone.

Proof of Concept

• Navigate to the following contract function.

https://github.com/code-423n4/2021-10-badgerdao/blob/9d4734becebd729299f154c0cfa1d3a7f06cccfb/contracts/WrappedIbbtc.sol#L38

https://github.com/code-423n4/2021-10-badgerdao/blob/9d4734becebd729299f154c0cfa1d3a7f06cccfb/contracts/WrappedIbbtcEth.sol#L37

• The attacker can initialize the contract before the legitimate deployer, hoping that the victim continues to use the same contract. In the best case for the victim, they notice it and have to redeploy their contract costing gas.

Tools Used

None

Recommended Mitigation Steps

Use the constructor to initialize non-proxied contracts. For initializing proxy contracts deploy contracts using a factory contract that immediately calls initialize after deployment or make sure to call it immediately after deployment and verify the transaction succeeded.

Handle

defsec

Vulnerability details

Impact

Since the _pendingGovernance parameter in the setPendingGovernance are used to add governance. In the state variable , proper check up should be done , other wise error in these state variable can lead to redeployment of contract.

Proof of Concept

1. Navigate to the following contract functions.

"https://github.com/code-423n4/2021-10-badgerdao/blob/9d4734becebd729299f154c0cfa1d3a7f06cccfb/contracts/WrappedIbbtcEth.sol#L50"

"https://github.com/code-423n4/2021-10-badgerdao/blob/9d4734becebd729299f154c0cfa1d3a7f06cccfb/contracts/WrappedIbbtc.sol#L49"

2. Adding zero address into the pending governance leads to failure of governor only functions.

Tools Used

Code Review

Recommended Mitigation Steps

Add proper zero address validation.

Handle

defsec

Vulnerability details

Impact

After pragma version, 0.7.0, the contract should use block.timestamp.

Proof of Concept

1. Navigate to the following contract.

"https://github.com/code-423n4/2021-10-badgerdao/blob/main/contracts/WrappedIbbtcEth.sol#L74"

2. Now is used instead of block.timestamp.

Tools Used

None

Recommended Mitigation Steps

It is recommended to use block.timestamp instead of now.

Handle

defsec

Vulnerability details

Impact

Using newer compiler versions and the optimizer gives gas optimizations and additional safety checks are available for free.

The advantages of versions 0.8.* over <0.8.0 are:

• Safemath by default from 0.8.0 (can be more gas efficient than library based safemath.)
• Low level inliner : from 0.8.2, leads to cheaper runtime gas. Especially relevant when the contract has small functions. For example, OpenZeppelin libraries typically have a lot of small helper functions and if they are not inlined, they cost an additional 20 to 40 gas because of 2 extra jump instructions and additional stack operations needed for function calls.
• Optimizer improvements in packed structs: Before 0.8.3, storing packed structs, in some cases used an additional storage read operation. After EIP-2929, if the slot was already cold, this means unnecessary stack operations and extra deploy time costs. However, if the slot was already warm, this means additional cost of 100 gas alongside the same unnecessary stack operations and extra deploy time costs.
• Custom errors from 0.8.4, leads to cheaper deploy time cost and run time cost. Note: the run time cost is only relevant when the revert condition is met. In short, replace revert strings by custom errors.

Proof of Concept

1. The contest repository contracts contain pragma 0.6.12^. The contracts pragma version should be updated to 0.8.4.

(https://github.com/code-423n4/2021-10-badgerdao/blob/main/contracts/WrappedIbbtc.sol#L2 && ) (https://github.com/code-423n4/2021-10-badgerdao/blob/main/contracts/ICoreOracle.sol#L3) (https://github.com/code-423n4/2021-10-badgerdao/blob/main/contracts/ICore.sol#L3) (https://github.com/code-423n4/2021-10-badgerdao/blob/main/contracts/WrappedIbbtcEth.sol#L2)

Tools Used

None

Recommended Mitigation Steps

Consider to upgrade pragma to at least 0.8.4.

Handle

defsec

Vulnerability details

Impact

On several locations in the code precautions are taken not to divide by 0, because this will revert the code. However on some locations this isn’t done.

Especially in the balanceToShares function div(pricePerShare) which isn’t checked.

That will cause to revert on the transfer and transferFrom function. Oracle pricePerShare variable should be cheked on the balance calculation.

Proof of Concept

1. Navigate to the following contracts,

"https://github.com/code-423n4/2021-10-badgerdao/blob/9d4734becebd729299f154c0cfa1d3a7f06cccfb/contracts/WrappedIbbtcEth.sol#L156"

"https://github.com/code-423n4/2021-10-badgerdao/blob/9d4734becebd729299f154c0cfa1d3a7f06cccfb/contracts/WrappedIbbtc.sol#L148"

2. If oracle fails, the pricePerShare variable will be equal to zero therefore div by zero will occur.

Tools Used

Review

Recommended Mitigation Steps

Recommend making sure division by 0 won’t occur by checking the variables beforehand and handling this edge case.