period can be set to 0 even when constructor disallows it

Summary

period can be set to zero, no challenge period for a proposal.

Vulnerability Detail

In constructor of TelcoinDistributor.sol, the period is check to not be equal to zero.

    constructor(
        IERC20 telcoin,
        uint256 period,
        IERC721 council
    ) Ownable(_msgSender()) {
        // verifies no zero values were used
        require(
            address(telcoin) != address(0) &&
                address(council) != address(0) &&
>               period != 0,
            "TelcoinDistributor: cannot intialize to zero"
        );

This period can be changed in setChallengePeriod(). There is no zero-check.

   function setChallengePeriod(uint256 newPeriod) public onlyOwner {
        //update period
>       challengePeriod = newPeriod;
        // Emitting an event for new period
        emit ChallengePeriodUpdated(challengePeriod);
    }

Impact

There can be zero challenge period. Normally disallowing 0 value is not an issue but since the council members are only semi-trusted, having 0 challenge period means that malicious council members can withdraw the owner's funds without any contestations.

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin-cryptostaker2/blob/475e5c92315d0b6cc1aa185a856fb8c24d993040/telcoin-audit/contracts/protocol/core/TelcoinDistributor.sol#L210-L215

Tool used

Manual Review

Recommendation

Recommend checking for zero value in setChallengePeriod() as well.

Duplicate of #127

sherlock-audit / 2024-01-telcoin-judging

Jaraxxus - period can be set to 0 even when constructor disallows it #183