search

code-423n4 / 2021-08-yield-findings

1 stars 0 forks source link

lack of zero address validation in constructor #40

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

JMukesh

Vulnerability details

Impact

since the parameter in constructor are used to initialize the state variable proper input validation should be there otherwise error in these state variable can be lead to redeployment of contract

Proof of Concept

https://github.com/code-423n4/2021-08-yield/blob/4dc46470e616dd0cbd9db9b4742e36c4d809e02c/contracts/utils/EmergencyBrake.sol#L34

https://github.com/code-423n4/2021-08-yield/blob/4dc46470e616dd0cbd9db9b4742e36c4d809e02c/contracts/utils/TimeLock.sol#L29

Tools Used

manual review

Recommended Mitigation Steps

add zero address validation

alcueca commented 3 years ago

I disagree. Validating that an address is not zero doesn't protect against entering the wrong address, so it is at best a very partial protection.

As with all contract building, the right approach is to script the contract deployment, with input parameters in files. A testing file should be included that verifies the deployment was correct. The deployment codebase is then peer-reviewed. Finally, the deployment is first done in a mainnet fork and tested. Only then is done on mainnet and tested.

ghoul-sol commented 3 years ago

Same as for #52, it's non-critical at best