GalloDaSballo
commented
2 years ago Ack but nothing special compared to the other 50 qas
Open code423n4 opened 2 years ago
GalloDaSballo
commented
2 years ago Ack but nothing special compared to the other 50 qas
using old solidity version
https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L3
recommend using a more up to date version of solidity
missing checks for zero address
description
Checking addresses against zero-address during initialization or during setting is a security best-practice. However, such checks are missing in address variable initializations/changes in many places.
Impact: Allowing zero-addresses will lead to contract reverts and force redeployments if there are no setters for such address variables.
findings
https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L59
use of magic numbers
description
constants should be declared rather than use magic numbers
findings
https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L205
open TODOs
description
Open TODOs can hint at programming or architectural errors that still need to be fixed.
Recommend resolving the TODO and bubble up the error.
findings
https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L284 https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L422
missing/incomplete NATSPEC
description
function parameters are not documented with proper natspec
anyone can send ETH
the receive() function has the following comment
however anyone can send ETH to this contract as long as it is claiming
https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L435-L438