Closed code423n4 closed 2 years ago
Low
Missing checks for zero addresses might lead to loss of funds, failed transactions and can break the protocol functionality.
MyStrategy.sol:
MyStrategy.sol
newBribesProcessor
Manual Review / VSCode
It is recommended to add zero address checks for listed parameters.
Contract is missing emitting events for critical functions. Lack of events makes it difficult for off-chain applications to monitor the protocol.
setWithdrawalSafetyCheck
setProcessLocksOnReinvest
setBribesProcessor
sweepRewardToken
_harvest
claimBribesFromHiddenHand
reinvest
manualSendAuraToVault
_sendBadgerToTree
It is recommended to add missing events to listed functions.
Non-Critical
Events should index addresses which helps off-chain applications in monitoring the protocol.
token
It is recommended to add indexing to address type parameters.
address
Function safeApprove has been deprecated in favor of safeIncreaseAllowance() and safeDecreaseAllowance().
safeApprove
safeIncreaseAllowance()
safeDecreaseAllowance()
MyStrategy.sol:65: AURA.safeApprove(address(LOCKER), type(uint256).max); MyStrategy.sol:67: AURABAL.safeApprove(address(BALANCER_VAULT), type(uint256).max); MyStrategy.sol:68: WETH.safeApprove(address(BALANCER_VAULT), type(uint256).max);
It is recommended to consider using safeIncreaseAllowance() and safeDecreaseAllowance() functions instead of safeApprove.
Contract is missing natspec comments which makes code more difficult to read and prone to errors.
@param _vault
@param delegate
@param newWithdrawalSafetyCheck
@param newProcessLocksOnReinvest
@param newBribesProcessor
@param token
@param tokens[]
@param _amount
@return string
@return bool
@return balance
@return rewards[]
@return protectedTokens
@return _amount
@return harvested
@param hiddenHandDistributor
_claims
@return toDeposit
@param amount
It is recommended to add missing natspec comments.
Dup of #52
1. Missing zero address checks
Risk
Low
Impact
Missing checks for zero addresses might lead to loss of funds, failed transactions and can break the protocol functionality.
Proof of Concept
MyStrategy.sol
:newBribesProcessor
- https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L98Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to add zero address checks for listed parameters.
2. Missing events
Risk
Low
Impact
Contract is missing emitting events for critical functions. Lack of events makes it difficult for off-chain applications to monitor the protocol.
Proof of Concept
MyStrategy.sol
:setWithdrawalSafetyCheck
function event - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L86setProcessLocksOnReinvest
function event - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L92setBribesProcessor
function event - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L98sweepRewardToken
function event - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L107_harvest
function event - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L219claimBribesFromHiddenHand
function event - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L288reinvest
function event - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L353manualSendAuraToVault
function event - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L379_sendBadgerToTree
function event - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L428Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to add missing events to listed functions.
3. Missing indexing for events
Risk
Non-Critical
Impact
Events should index addresses which helps off-chain applications in monitoring the protocol.
Proof of Concept
MyStrategy.sol
:token
address - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L51Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to add indexing to
address
type parameters.4. Deprecated safeApprove
Risk
Non-Critical
Impact
Function
safeApprove
has been deprecated in favor ofsafeIncreaseAllowance()
andsafeDecreaseAllowance()
.Proof of Concept
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to consider using
safeIncreaseAllowance()
andsafeDecreaseAllowance()
functions instead ofsafeApprove
.5. Missing natspec comments
Risk
Non-Critical
Impact
Contract is missing natspec comments which makes code more difficult to read and prone to errors.
Proof of Concept
MyStrategy.sol
:@param _vault
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L53-L56@param delegate
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L78-L79@param newWithdrawalSafetyCheck
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L86@param newProcessLocksOnReinvest
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L92@param newBribesProcessor
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L98@param token
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L107@param tokens[]
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L116@param _amount
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L171@return string
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L125-L126@return string
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L130-L131@return bool
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L135-L136@return balance
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L140-L141@return rewards[]
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L149@return protectedTokens
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L158-L161@param _amount
,@return _amount
comments - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L192-L194@return harvested
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L219@param hiddenHandDistributor
,_claims
comments - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L288@return toDeposit
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L353@param token
,@param amount
comments - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L421@param amount
comment - https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L428Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to add missing natspec comments.