Wrong implementation of function addVoteEscrow() in RewardDistributor.sol

[code423n4]

Lines of code

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L298-L305

Vulnerability details

Impact

• Denial of Service because ve always is address(0)

Proof of concept

There is no initial setup for ve variable in contract RewardDistributor. So if we want to set ve variable, we will need to call function addVoteEscrow().

function addVoteEscrow(address _voteEscrow) external onlyOwner {
    if (address(ve) == address(0)) {
        ve = VE(pendingVoteEscrow);
    } else {
        voteEscrowEnableDate = block.timestamp + 1 days;
        pendingVoteEscrow = _voteEscrow;
    }
}

Function addVoteEscrow() will check address(ve) == address(0) or not. Cause there is no initialization for ve, it will set ve = VE(pendingVoteEscrow). But at this time, pendingVoteEscrow is address(0) too.
==> ve == address(0) forever

Tools Used

Manual review

Recommended Mitigation Steps

change function to

function addVoteEscrow(address _voteEscrow) external onlyOwner {
    if (address(ve) == address(0)) {
        ve = VE(_voteEscrow);
    } else {
        voteEscrowEnableDate = block.timestamp + 1 days;
        pendingVoteEscrow = _voteEscrow;
    }
}

[okkothejawa]

Duplicate of #611