Description

If a majority proposal is not pending or has expired, the authority can propose a new one to override it.

The function proposeMajorityMetavestAmendment checks if the proposal is pending and the time is less than block.timestamp. If so, the transaction is reverted.

function proposeMajorityMetavestAmendment(
    string memory setName,
    bytes4 _msgSig,
    bytes calldata _callData
) external onlyAuthority {
    //if the majority proposal is already pending and not expired, revert
!   if (functionToSetMajorityProposal[_msgSig][setName].isPending && block.timestamp > functionToSetMajorityProposal[_msgSig][setName].time)
        revert MetaVesTController_AmendmentAlreadyPending();
    // [...]
}

The functionToSetMajorityProposal[_msgSig][setName].time is the proposal creation time. So, the statement block.timestamp > functionToSetMajorityProposal[_msgSig][setName].time will always return true, unless in the same block.

Impact

Because a proposal remains pending after its creation to update multiple grants, if the function cannot correctly identify an expired proposal, it cannot create a new proposal with the same msg.sig and the same set.

The following proof-of-concept script demonstrates that a transaction will be reverted even if the proposal has expired:

function testAuditProposeMajorityMetavestAmendmentExpire() public {
    address mockAllocation = createDummyVestingAllocation();
    bytes4 msgSig = bytes4(keccak256("updateMetavestTransferability(address,bool)"));
    bytes memory callData = abi.encodeWithSelector(msgSig, mockAllocation, true);

    vm.prank(authority);
    controller.addMetaVestToSet("testSet", mockAllocation);

    vm.prank(authority);
    controller.proposeMajorityMetavestAmendment("testSet", msgSig, callData);

    // proposal expired
    uint256 AMENDMENT_TIME_LIMIT = 604800;
    vm.warp(block.timestamp + AMENDMENT_TIME_LIMIT + 1);

    vm.prank(authority);
    vm.expectRevert(
        metavestController
        .MetaVesTController_AmendmentAlreadyPending
        .selector
    );
    controller.proposeMajorityMetavestAmendment("testSet", msgSig, callData);
}

Recommendations

Consider using the statement block.timestamp < functionToSetMajorityProposal[_msgSig][setName].time + AMENDMENT_TIME_LIMIT to ensure the proposal has not expired.

Remediation

This issue has been acknowledged by MetaLeX Labs, Inc, and a fix was implemented in commit c62b8c25↗.