code-423n4 / 2022-11-stakehouse-findings

1 stars 1 forks source link

claimRewards() Logic error #297

Closed code423n4 closed 1 year ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/StakingFundsVault.sol#L215

Vulnerability details

Impact

StakingFundsVault#claimRewards() will trigger _claimFundsFromSyndicateForDistribution(), but only if it is the first and isNoLongerPartOfSyndicate(), it does not make sense, if not in the first, the second is also should be able to trigger

Proof of Concept

function claimRewards(
    address _recipient,
    bytes[] calldata _blsPubKeys
) external nonReentrant {

    for (uint256 i; i < _blsPubKeys.length; ++i) {

     //***@audit only first and first is "isNoLongerPartOfSyndicate"***/
      if (i == 0 && !Syndicate(payable(liquidStakingNetworkManager.syndicate())).isNoLongerPartOfSyndicate(_blsPubKeys[i])) {
            _claimFundsFromSyndicateForDistribution(
                liquidStakingNetworkManager.syndicate(),
                _blsPubKeys
            );

    }

Tools Used

Recommended Mitigation Steps

    function claimRewards(
        address _recipient,
        bytes[] calldata _blsPubKeys
    ) external nonReentrant {
+       bool alwayDistribution;
        for (uint256 i; i < _blsPubKeys.length; ++i) {
            require(
                liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPubKeys[i]) == false,
                "Unknown BLS public key"
            );

            // Ensure that the BLS key has its derivatives minted
            require(
                getAccountManager().blsPublicKeyToLifecycleStatus(_blsPubKeys[i]) == IDataStructures.LifecycleStatus.TOKENS_MINTED,
                "Derivatives not minted"
            );
-           if (i == 0 && !Syndicate(payable(liquidStakingNetworkManager.syndicate())).isNoLongerPartOfSyndicate(_blsPubKeys[i])) {
+           if (!alwayDistribution && !Syndicate(payable(liquidStakingNetworkManager.syndicate())).isNoLongerPartOfSyndicate(_blsPubKeys[i])) {
+               alwayDistribution = true;            

                _claimFundsFromSyndicateForDistribution(
                    liquidStakingNetworkManager.syndicate(),
                    _blsPubKeys
                );

....
        }
dmvt commented 1 year ago

The warden failed to describe a vulnerability, impact, negative result, or otherwise prove that the issue reported exists.

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Insufficient proof