sherlock-audit / 2023-10-perennial-judging

11 stars 7 forks source link

kaancaglan - Missing checks for `address(0)` in functions #17

Closed sherlock-admin2 closed 10 months ago

sherlock-admin2 commented 10 months ago

kaancaglan

medium

Missing checks for address(0) in functions

Summary

Smart contracts in the given repository lack the necessary checks for the zero address during initialization. This oversight can lead to critical issues where tokens or rights may be unintentionally assigned to an address that can never be used, leading to permanent loss of assets or access rights.

Vulnerability Detail

The constructors and initializers of the contracts do not validate against the zero address. This can result in the permanent loss of tokens if they are sent to the zero address by mistake, as these tokens cannot be retrieved. Furthermore, setting critical addresses, such as marketFactory, vaultFactory, batcher, and reserve to the zero address could cripple the contract functionality.

Impact

The impact of not checking for the zero address is medium as it could lead to asset loss or a halt in contract functionality. However, the likelihood is considered low, assuming careful deployment practices. Nonetheless, the risk should not be overlooked.

Code Snippet

Click to show findings ```solidity Path: ./perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol 71: marketFactory = marketFactory_; // @audit-issue 72: vaultFactory = vaultFactory_; // @audit-issue 73: batcher = batcher_; // @audit-issue 74: reserve = reserve_; // @audit-issue ``` *GitHub*: [71](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L71-L71), [72](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L72-L72), [73](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L73-L73), [74](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L74-L74) ```solidity Path: ./perennial-v2/packages/perennial/contracts/Market.sol 68: token = definition_.token; // @audit-issue 69: oracle = definition_.oracle; // @audit-issue 70: payoff = definition_.payoff; // @audit-issue 97: beneficiary = newBeneficiary; // @audit-issue 104: coordinator = newCoordinator; // @audit-issue ``` *GitHub*: [68](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial/contracts/Market.sol#L68-L68), [69](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial/contracts/Market.sol#L69-L69), [70](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial/contracts/Market.sol#L70-L70), [97](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial/contracts/Market.sol#L97-L97), [104](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial/contracts/Market.sol#L104-L104) ```solidity Path: ./perennial-v2/packages/perennial-oracle/contracts/keeper/KeeperFactory.sol 105: oracleFactory = oracleFactory_; // @audit-issue ``` *GitHub*: [105](https://github.com/sherlock-audit/2023-10-perennial/blob/main/./perennial-v2/packages/perennial-oracle/contracts/keeper/KeeperFactory.sol#L105-L105)

Tool used

Manual Review - Used my own static analyzer (still in development)

Recommendation

It is strongly recommended to implement checks to prevent the zero address from being set during the initialization of contracts. This can be achieved by adding require statements that ensure address parameters are not the zero address.

sherlock-admin2 commented 10 months ago

3 comment(s) were left on this issue during the judging contest.

panprog commented:

low, if incorrect address is given by user - it's user error, which is invalid by Sherlock rules

tsvetanovv commented:

Information

chainNue commented:

missing check zero address is not valid finding in sherlock https://docs.sherlock.xyz/audits/judging/judging#vii.-list-of-issue-categories-that-are-not-considered-valid