FEI and TRIBE Redemption contest - hyh's results

Lines of code

https://github.com/code-423n4/2022-09-tribe/blob/769b0586b4975270b669d7d1581aa5672d6999d5/contracts/shutdown/fuse/RariMerkleRedeemer.sol#L88-L97

Vulnerability details

User can rewrite the signature stored any number of times.

That's a violation of the desired system logic with no immediate loss of funds, so placing severity to be medium.

Proof of Concept

signAndClaim() don't control the signing status, so it can be run multiple times:

https://github.com/code-423n4/2022-09-tribe/blob/769b0586b4975270b669d7d1581aa5672d6999d5/contracts/shutdown/fuse/RariMerkleRedeemer.sol#L88-L97

    function signAndClaim(
        bytes calldata signature,
        address[] calldata cTokens,
        uint256[] calldata amounts,
        bytes32[][] calldata merkleProofs
    ) external override nonReentrant {
        // both sign and claim/multiclaim will revert on invalid signatures/proofs
        _sign(signature);
        _multiClaim(cTokens, amounts, merkleProofs);
    }

I.e. a user can rerun signAndClaim many times for example with empty cTokens and amounts arrays as _multiClaim() will be a noop in this case:

https://github.com/code-423n4/2022-09-tribe/blob/769b0586b4975270b669d7d1581aa5672d6999d5/contracts/shutdown/fuse/RariMerkleRedeemer.sol#L183-L198

    // User provides the cTokens & the amounts they should get, and it is verified against the merkle root for that cToken (for each cToken provided)
    // Should set the user's claim amount in the claims mapping for each cToken provided
    function _multiClaim(
        address[] calldata _cTokens,
        uint256[] calldata _amounts,
        bytes32[][] calldata _merkleProofs
    ) internal virtual {
        require(_cTokens.length == _amounts.length, "Number of cTokens and amounts must match");
        require(_cTokens.length == _merkleProofs.length, "Number of cTokens and merkle proofs must match");

        for (uint256 i = 0; i < _cTokens.length; i++) {
            _claim(_cTokens[i], _amounts[i], _merkleProofs[i]);
        }

        // no events needed here, they happen in _claim
    }

Recommended Mitigation Steps

Consider adding the hasNotSigned modifier:

https://github.com/code-423n4/2022-09-tribe/blob/769b0586b4975270b669d7d1581aa5672d6999d5/contracts/shutdown/fuse/RariMerkleRedeemer.sol#L88-L97

    function signAndClaim(
        bytes calldata signature,
        address[] calldata cTokens,
        uint256[] calldata amounts,
        bytes32[][] calldata merkleProofs
-   ) external override nonReentrant {
+   ) external override hasNotSigned nonReentrant {
        // both sign and claim/multiclaim will revert on invalid signatures/proofs
        _sign(signature);
        _multiClaim(cTokens, amounts, merkleProofs);
    }

This way the behavior will be aligned with all other helper functions