sherlock-audit / 2024-01-rio-vesting-escrow-judging

3 stars 2 forks source link

0xBhumii - Missing zero address check #87

Closed sherlock-admin2 closed 9 months ago

sherlock-admin2 commented 9 months ago

0xBhumii

medium

Missing zero address check

Summary

In VestingEscrowFactory contract in constructor zero address check is missing for votingAdaptor

Vulnerability Detail

In the VestingEscrowFactory contract, the constructor is missing a check for the zero address (address(0)) when setting the initial value for the votingAdaptor. This omission may have several consequences:

  1. Potential Deployment Issues: The votingAdaptor is intended to be a crucial part of the functionality of the contract, not having a zero address check during the contract deployment could lead to unexpected behavior. The contract might be deployed with an invalid or uninitialized votingAdaptor address.
  2. Security Risks: Without a zero address check, the contract may be deployed with a votingAdaptor set to address zero, which is a common pattern for an uninitialized address. This can create security risks, as certain operations depending on the votingAdaptor may not behave as expected or may lead to vulnerabilities.

Impact

Without a zero address check contract might be deployed with an invalid or uninitialized votingAdaptor address.

Code Snippet

https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/VestingEscrowFactory.sol#L30C1-L41C6

    constructor(address _vestingEscrowImpl, address _token, address _owner, address _manager, address _votingAdaptor) {
        if (_vestingEscrowImpl == address(0)) revert INVALID_VESTING_ESCROW_IMPL();
        if (_token == address(0)) revert INVALID_TOKEN();
        if (_owner == address(0)) revert INVALID_OWNER();

        vestingEscrowImpl = _vestingEscrowImpl;
        token = _token;
        manager = _manager;
        votingAdaptor = _votingAdaptor;

        _transferOwnership(_owner);
    } 

Tool used

Manual Review

Recommendation

To address this issue, it is recommended to include a zero address check in the constructor when setting the votingAdaptor. For example:

if (_votingAdaptor == address(0)) revert INVALID_VOTING_ADAPTOR();
votingAdaptor = _votingAdaptor;

This check ensures that the provided votingAdaptor address is not the zero address, helping to prevent potential issues related to uninitialized or invalid addresses. Additionally, it provides better clarity on the expected behavior during contract deployment.

Duplicate of #109

solimander commented 9 months ago

Invalid.

sherlock-admin2 commented 9 months ago

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

pratraut commented:

'invalid as owner can set it and function which need votingAdaptor have an modifier to check its not NULL address'