sherlock-audit / 2024-07-sense-points-marketplace-judging

2 stars 0 forks source link

Large Onyx Butterfly - `collectFees` can create a massive amount of events to the off chain system #210

Closed sherlock-admin4 closed 2 weeks ago

sherlock-admin4 commented 2 weeks ago

Large Onyx Butterfly

Low/Info

collectFees can create a massive amount of events to the off chain system

Summary

Since part of the protocol relies on an off chain system to interact externally, the function collectFees can increase exponentially the amount of events to be analyzed.

Root Cause

In https://github.com/sherlock-audit/2024-07-sense-points-marketplace/blob/main/point-tokenization-vault/contracts/PointTokenVault.sol#L343-L358 the external open function allows for pTokenFeeAcc and rewardTokenFeeAcc to be mint into the feeCollector account and the FeesCollected is emitted.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

  1. User interacts with the collectFees function inputting just the _pointsId of the respective program.

Impact

Regardless of the amount stored in the pTokenFeeAcc and rewardTokenFeeAcc, conditions of mint will be checked, and in the event of 0 value on both accumulators, the FeesCollected will still be emitted by this interaction.

PoC

No response

Mitigation

Consider adding a check on the collectFees function to only allow interactions if the accumulated values of both variables are different of 0.

function collectFees(bytes32 _pointsId) external {
        (uint256 pTokenFee, uint256 rewardTokenFee) = (pTokenFeeAcc[_pointsId], rewardTokenFeeAcc[_pointsId]);

        require(pTokenFee != 0 && rewardTokenFee != 0) // here

        if (pTokenFee > 0) {
            pTokens[_pointsId].mint(feeCollector, pTokenFee);
            pTokenFeeAcc[_pointsId] = 0;
        }

        if (rewardTokenFee > 0) {
            // There will only be a positive rewardTokenFee if there are reward tokens in this contract available for transfer.
            redemptions[_pointsId].rewardToken.safeTransfer(feeCollector, rewardTokenFee);
            rewardTokenFeeAcc[_pointsId] = 0;
        }

        emit FeesCollected(_pointsId, feeCollector, pTokenFee, rewardTokenFee);
    }