Venus Prime - 0xWaitress's results

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/PrimeLiquidityProvider.sol#L216-L225

Vulnerability details

Impact

balanceOf of reward Tokens is meant to be strictly larger than accrued to in PrimeLiquidityProvider, however the owner has the ability to sweep reward tokens which could revert the accrue function

If the owner sweep the reward Tokens using sweepToken()

    function sweepToken(IERC20Upgradeable token_, address to_, uint256 amount_) external onlyOwner {
        uint256 balance = token_.balanceOf(address(this));
        if (amount_ > balance) {
            revert InsufficientBalance(amount_, balance);
        }

        emit SweepToken(address(token_), to_, amount_);

        token_.safeTransfer(to_, amount_);
    }

Then the balanceOf rewardTokens would not be larger than the accruedAmount tokenAmountAccrued

during accrueTokens, balanceDiff would have underflow.

    function accrueTokens(address token_) public {
        _ensureZeroAddress(token_);

        _ensureTokenInitialized(token_);

        uint256 blockNumber = getBlockNumber();
        uint256 deltaBlocks = blockNumber - lastAccruedBlock[token_];

        if (deltaBlocks > 0) {
            uint256 distributionSpeed = tokenDistributionSpeeds[token_];
            uint256 balance = IERC20Upgradeable(token_).balanceOf(address(this));

            uint256 balanceDiff = balance - tokenAmountAccrued[token_];
            if (distributionSpeed > 0 && balanceDiff > 0) {
                uint256 accruedSinceUpdate = deltaBlocks * distributionSpeed;
                uint256 tokenAccrued = (balanceDiff <= accruedSinceUpdate ? balanceDiff : accruedSinceUpdate);

                tokenAmountAccrued[token_] += tokenAccrued;
                emit TokensAccrued(token_, tokenAccrued);
            }

            lastAccruedBlock[token_] = blockNumber;
        }
    }

Impact: accrueTokens, which is an essential flow for releaseFund and _setTokenDistributionSpeed would be reverted if owner sweep the reward tokens.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Tools Used

Recommended Mitigation Steps

if a token has a non-zero distribution speed, validate and revert the sweepToken.

Assessed type

Context

[G-1] pendingScoreUpdates can be updated at the end of updateScores instead of using pendingScoreUpdates-- at every loop, with the use of an intermediate variable in memory.

    function updateScores(address[] memory users) external {
        if (pendingScoreUpdates == 0) revert NoScoreUpdatesRequired();
        if (nextScoreUpdateRoundId == 0) revert NoScoreUpdatesRequired();

        for (uint256 i = 0; i < users.length; ) {
            address user = users[i];

            if (!tokens[user].exists) revert UserHasNoPrimeToken();
            if (isScoreUpdated[nextScoreUpdateRoundId][user]) continue;

            address[] storage _allMarkets = allMarkets;
            for (uint256 j = 0; j < _allMarkets.length; ) {
                address market = _allMarkets[j];
                _executeBoost(user, market);
                _updateScore(user, market);

                unchecked {
                    j++;
                }
            }

            pendingScoreUpdates--;
            isScoreUpdated[nextScoreUpdateRoundId][user] = true;

            unchecked {
                i++;
            }

            emit UserScoreUpdated(user);
        }
    }

Recommendation

move pendingScoreUpdates-- outside of the loop and replace it with the use of a counter.

uint256 counter = 0;
...
---            pendingScoreUpdates--;
+++            counter++;
...
pendingScoreUpdates -= counter;