Udsen - `address(0)` CHECK IS NOT PERFORMED FOR THE `address` VARIABLES PASSED INTO THE `initiazlize()` FUNCTIONS OF THE IMPLEMENTATION CONTRACTS.

[github-actions[bot]]

Udsen

false

address(0) CHECK IS NOT PERFORMED FOR THE address VARIABLES PASSED INTO THE initiazlize() FUNCTIONS OF THE IMPLEMENTATION CONTRACTS.

Summary

The four types of bounty implementation contracts, namely AtomicBountyV1, OngoinBountyV1, TieredFixedBountyV1 and TieredPercentageBountyV1 contracts in their initialize() functions call the following init functions.

    __OnlyOpenQ_init(_openQ); 
    __ClaimManagerOwnable_init(_claimManager); 
    __DepositManagerOwnable_init(_depositManager); 

The _openQ, _claimManager and _depositManager are passed into the initialize() function as address parameters.

Vulnerability Detail

The _openQ, _claimManager and _depositManager addresses passed into the initialize() are not checked for the address(0) before initialization is performed.

    __OnlyOpenQ_init(_openQ); 
    __ClaimManagerOwnable_init(_claimManager); 
    __DepositManagerOwnable_init(_depositManager); 

Above three init functions defines the private storage variable for onlyOpenQ, onlyClaimManager and onlyDepositManager modifiers. There are multiple functions controlled by above three modifiers. Hence extra steps should be taken to check for the zero address of above _openQ, _claimManager and _depositManager addresses when they are passed into the initialize() function.

Impact

If the _openQ, _claimManager and _depositManager addresses are assigned with address(0) it will revert all the functions controlled by _openQ, _claimManager and _depositManager modifiers during run time. Which is not the intended functioning of the protocol.

Code Snippet

    function initialize(
        string memory _bountyId,
        address _issuer,
        string memory _organization,
        address _openQ,
        address _claimManager,
        address _depositManager,
        OpenQDefinitions.InitOperation memory _operation
    ) external initializer {
        require(bytes(_bountyId).length != 0, Errors.NO_EMPTY_BOUNTY_ID);
        require(bytes(_organization).length != 0, Errors.NO_EMPTY_ORGANIZATION);

        __ReentrancyGuard_init();

        __OnlyOpenQ_init(_openQ); 
        __ClaimManagerOwnable_init(_claimManager);
        __DepositManagerOwnable_init(_depositManager);

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/AtomicBountyV1.sol#L27-L43

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/OngoingBountyV1.sol#L27-L43

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/TieredFixedBountyV1.sol#L27-L43

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/TieredPercentageBountyV1.sol#L26-L42

Tool used

VS Code and Manual Review

Recommendation

Add the require() statement to check for the address(0) for the _openQ, _claimManager and _depositManager address variables when passed into the initialize() function of the implementation contracts.

    function initialize(
        string memory _bountyId,
        address _issuer,
        string memory _organization,
        address _openQ,
        address _claimManager,
        address _depositManager,
        OpenQDefinitions.InitOperation memory _operation
    ) external initializer {
        require(bytes(_bountyId).length != 0, Errors.NO_EMPTY_BOUNTY_ID);
        require(bytes(_organization).length != 0, Errors.NO_EMPTY_ORGANIZATION);

    require(_openQ != address(0), "_openQ can not be zero address")
        require(_claimManager != address(0), "_claimManager can not be zero address")
    require(_depositManager != address(0), "_depositManager can not be zero address")

        __ReentrancyGuard_init();

        __OnlyOpenQ_init(_openQ); 
        __ClaimManagerOwnable_init(_claimManager);
        __DepositManagerOwnable_init(_depositManager);