Usual sUSD0 Yield Bearing Vault Security Review Report

The function _update is part of the ERC20 implementation and handles the transfers. The SUsd0 contract overwrites this function to add blacklist functionality. It will check both the from and to address for blacklisted status.

The blacklist check can be skipped for address(0), which will happen on a mint and a burn. One SLOAD would be saved every time someone deposits to or withdraws from the vault.

function _update(address from, address to, uint256 amount)
    internal
    override(ERC20Upgradeable)
    whenNotPaused
{
    SUsd0StorageV0 storage $ = _susd0StorageV0();
    if ($.isBlacklisted[from] || $.isBlacklisted[to]) {
        revert Blacklisted();
    }
    super._update(from, to, amount);
}

Optimize the check to skip address(0), for example:

if ((from != address(0) && $.isBlacklisted[from])
    || (to != address(0) && $.isBlacklisted[to])) {
    revert Blacklisted();
}

The distributeAccruingDT() function is used to distribute newly minted usd0 tokens to the $.rewardClaimForAccruingDT contract by calling the startYieldDistribution() function after minting:

$.usd0.mint(address($.rewardClaimForAccruingDT), amount);
// Start yield distribution
$.rewardClaimForAccruingDT.startYieldDistribution(
    amount, block.timestamp, block.timestamp + ONE_DAY
);

This only works correctly when the asset of the rewardClaimForAccruingDT contract is $.usd0. Otherwise, the startYieldDistribution() function will revert because the incorrect token was minted.

However, there is no check to verify that the asset is correct when setting $.rewardClaimForAccruingDT in the RevenueDistributionModule in the initialize() function or the setRewardClaimForAccruingDT() function.

Whenever the $.rewardClaimForAccruingDT address is updated, its asset token should be verified to match $.usd0.

In the RevenueDistributionModule contract the configuration variable dailyAccruingYieldRate is set in the initializer from the parameter values. It is a rate expressed in micro bps (1_000_000) and it is correctly checked to be at most equal to the maximum value of 100%.

However, in the setter function setDailyAccruingYieldRate, this cap check is missing. The privileged role is able to set a rate that is higher than 100%.

function setDailyAccruingYieldRate(uint256 newDailyRate) external whenNotPaused {
    if (newDailyRate == 0) {
        revert AmountIsZero();
    }
    RevenueDistributionModuleStorageV0 storage $ = _revenueDistributionModuleStorageV0();
    $.registryAccess.onlyMatchingRole(OPERATOR_ADMIN_ROLE);
 
    if (newDailyRate == $.dailyAccruingYieldRate) {
        revert SameValue();
    }
 
    $.dailyAccruingYieldRate = newDailyRate;
 
    emit DailyAccruingYieldRateUpdated(newDailyRate);
}

We recommend to add a sanity check to setDailyAccruingYieldRate to not allow values greater than 100%.

In the _startYieldDistribution() function, there is a check to verify that startTime is not below $.periodFinish. However, startTime is already required to be greater than or equal to block.timestamp, and there is also a check to verify that block.timestamp is greater than or equal to $.periodFinish. Therefore, startTime cannot be below $.periodFinish, making this check unnecessary.

function _startYieldDistribution(uint256 yieldAmount, uint256 startTime, uint256 endTime)
    internal
    override
{
    IERC20 _asset = IERC20(asset());
 
    if (yieldAmount == 0) {
        revert ZeroYieldAmount();
    }
    if (startTime < block.timestamp) {
        revert StartTimeNotInFuture();
    }
    if (endTime <= startTime) {
        revert EndTimeNotAfterStartTime();
    }
 
    YieldDataStorage storage $ = _getYieldDataStorage();
 
    if (startTime < $.periodFinish) {
        revert StartTimeBeforePeriodFinish();
    }
    if (block.timestamp < $.periodFinish) {
        revert CurrentTimeBeforePeriodFinish();
    }
 
    _updateYield();
 
    uint256 periodDuration = endTime - startTime;
    uint256 newYieldRate =
        Math.mulDiv(yieldAmount, YIELD_PRECISION, periodDuration, Math.Rounding.Floor);
 
    if (_asset.balanceOf(address(this)) < $.totalDeposits + yieldAmount) {
        revert InsufficientAssetsForYield();
    }
 
    $.yieldRate = newYieldRate;
    $.periodStart = startTime;
    $.periodFinish = endTime;
    $.lastUpdateTime = startTime;
    $.isActive = true;
}

Remove the following check:

if (startTime < $.periodFinish) {
    revert StartTimeBeforePeriodFinish();
}