code-423n4 / 2022-06-badger-findings

0 stars 0 forks source link

QA Report #45

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Badger Vested Aura QA Report

Summary

The README.md could be updated to give better understanding.

Non-Critical

Missing zero address check for `bribesProcessor`

MyStrategy.sol:100
```
 ///@dev Change the contract that handles bribes
function setBribesProcessor(IBribesProcessor newBribesProcessor) external {
    _onlyGovernance();
    bribesProcessor = newBribesProcessor;
}
```
The bribeProcessor is not set in the initialize function, so it starts with zero address. Also, setBribesProcessor can set the bribesProcessor to the zero address. Although no amount can be transferred to zero address thanks to the usage of safeTransfer, sweepRewardToken and claimBribesFromHiddenHand will revert when the bribesProcessor is not set. To mitigate this, the bribesProcessor can be set in the initialize function and add zero address check to setBribesProcessor.

typo in comment

MyStrategy.sol:218

  ///      after claiming rewards or swapping are auto-compunded.

auto-compunded to auto-compounded

todo in comment

MyStrategy.sol:284

// TODO: Hardcode claim.account = address(this)?

GalloDaSballo commented 2 years ago

Missing zero address check for bribesProcessor

What if we don't want to have a bribesProcessor anymore?

ack rest

jack-the-pug commented 2 years ago

I'll upgrade Missing zero address check for bribesProcessor to High and that's a dup of #18