Farseen0/scar-pairs huggingface dataset:
MEDIUM severity: If MINTR is paused, SiloAMO will not be able to unwind its position. There are two situations when the SiloAMO will withdraw OHM from the Silo:

1. The OHM deposits in the Silo relative to borrows results in a utilization rate greater than `uopt`, and `update()` is called.

2. The admin or emergency roles of SiloAMO decide to reduce the position or (in the case of...

Vulnerable code:
function _withdraw(uint256 amount_) internal {
    // The OHM deposit will accrue interest over time leading to the potential to withdraw more OHM
    // than is tracked by the ohmDeployed value. This is fine, but we need to avoid underflow errors
    // and track the amount of OHM that has been burned from the circulating supply after being accrued
    // as interest
    if (ohmDeployed < amount_) circulatingOhmBurned += amount_ - ohmDeployed;
    ohmDeployed -= ohmDeployed > amount_ ? amount_ : ohmDeployed;

    // Withdraw OHM from Silo
    ISilo(market).withdraw(address(OHM), amount_, false);

    // Burn received OHM
    _burnOhm(amount_);

    emit Withdrawal(amount_);
}

function _burnOhm(uint256 amount_) internal {
    OHM.increaseAllowance(address(MINTR), amount_);
    MINTR.burnOhm(address(this), amount_);
}

modifier onlyWhileActive() {
    if (!active) revert MINTR_NotActive();
    _;
}
LOW severity: Yield can only be withdrawn to the admin address, not the treasury. When incentives are harvested using the `harvestYield()` function, the yield — which is paid in SILO tokens — is kept in the contract.

The only way to withdraw this ERC20 is to use the `sweepTokens()` function, which sends the balance to the admin.

However, as confirmed by the team, the yield...

Vulnerable code:
function sweepTokens(address token_) external onlyRole("lendingamo_admin") {
    ERC20(token_).transfer(msg.sender, ERC20(token_).balanceOf(address(this)));
}
LOW severity: Admin has unlimited power to mint new OHM. While the SiloAMO system appears to have many checks and balances, in the form of a permissionless `update()` function and a `maximumToDeploy` cap on OHM deployed, all these checks can be circumvented by a malicious admin.

While I don't see an incentive for the admin to do this, it is important...

Vulnerable code:
function setMaximumToDeploy(uint256 newMaximum_) external onlyRole("lendingamo_admin") {
    maximumToDeploy = newMaximum_;
}

function setRateModel(address newRateModel_) external onlyRole("lendingamo_admin") {
    rateModel = newRateModel_;
}

function setUpdateInterval(uint256 newInterval_) external onlyRole("lendingamo_admin") {
    updateInterval = newInterval_;
}
LOW severity: `update()` can be DOS'd by frontrunning with dust deposits and withdrawals. The `update()` function on the SiloAMO has the following check to protect against flash loans being used to manipulate the utilization rate:

This `utilizationData.interestRateTimestamp` is updated every time `_accrueInterest()` is called on the Silo. This occurs each time funds are deposited,...

Vulnerable code:
ISilo.UtilizationData memory utilizationData = ISilo(market).utilizationData(address(OHM));
if (utilizationData.interestRateTimestamp == block.timestamp)
    revert AMO_UpdateReentrancyGuard(address(this));
LOW severity: Obol fees will be applied retroactively to all non-distributed funds in the Splitter. When Obol decides to turn on fees, a call will be made to `ImmutableSplitController::updateSplit()`, which will take the predefined split parameters (the original user specified split with Obol's fees added in) and call `updateSplit()` to implement the change.

If we look at the code on...

Vulnerable code:
function updateSplit() external payable {
    if (msg.sender != owner()) revert Unauthorized();

    (address[] memory accounts, uint32[] memory percentAllocations) = getNewSplitConfiguration();

    ISplitMain(splitMain()).updateSplit(split, accounts, percentAllocations, uint32(distributorFee()));
}

function updateSplit(
    address split,
    address[] calldata accounts,
    uint32[] calldata percentAllocations,
    uint32 distributorFee
)
    external
    override
    onlySplitController(split)
    validSplit(accounts, percentAllocations, distributorFee)
{
    _updateSplit(split, accounts, percentAllocations, distributorFee);
}
LOW severity: LidoSplit can receive ETH, which will be locked in contract. Each new `LidoSplit` is deployed as a clone, which comes with a `receive()` function for receiving ETH.

However, the only function on `LidoSplit` is `distribute()`, which converts `stETH` to `wstETH` and transfers it to the `splitWallet`.

While this contract should only be used for Lido to pay...

Vulnerable code:
function testZach_CanReceiveEth() public {
    uint before = address(lidoSplit).balance;
    payable(address(lidoSplit)).transfer(1 ether);
    assertEq(address(lidoSplit).balance, before + 1 ether);
}
LOW severity: Upgrade to latest version of Solady to fix LibClone bug. In the recent [Solady audit](https://github.com/Vectorized/solady/blob/main/audits/cantina-solady-report.pdf), an issue was found the affects LibClone.

In short, LibClone assumes that the length of the immutable arguments on the clone will fit in 2 bytes. If it's larger, it overlaps other op...

Vulnerable code:
function testZach__CreateControllerSoladyBug() public {
    ImmutableSplitControllerFactory factory = new ImmutableSplitControllerFactory(address(9999));
    bytes32 deploymentSalt = keccak256(abi.encodePacked(uint256(1102)));
    address owner = address(this);

    address[] memory bigAccounts = new address[](28672);
    uint32[] memory bigPercentAllocations = new uint32[](28672);

    for (uint i = 0; i < 28672; i++) {
        bigAccounts[i] = address(uint160(i));
        if (i < 32) bigPercentAllocations[i] = 820;
        else bigPercentAllocations[i] = 34;
    }

    ImmutableSplitController controller = factory.createController(address(8888), owner, bigAccounts, bigPercentAllocations, 0, deploymentSalt);
    assert(address(controller) != address(0));
    assert(address(controller).code.length == 0);
}
MEDIUM severity: Optimizer will misallocate when StakeDAO is above full boost. On Page 5 of the whitepaper, it says:

> We can assume that when this balance is reached for a given gauge, neither Convex nor Stake DAO have maximum boost, which enables us to get rid of the min formula.

The `min` in the formula is intended to cap the boost at 2.5x. If the proportion of veCRV...

Vulnerable code:
boost = ((0.4 * 1) + (0.6 * 100 * 10%)) / 0.4 = 16x

convex _return= convex_boost * (1 - 0.17 + 0.05)
sd_return = sd_boost * (1 - 0.16)

convex_boost * 0.88 == 2.5 * 0.84
convex_boost = 2.386
MEDIUM severity: Deployment script mismatches gauges and reward distributors. In Deployment.s.sol, a list of gauges (Curve) and reward distributors (StakeDAO) are provided to migrate.

The gauge list is 162 elements long, but the reward distributor list is only 160 elements long, missing the corresponding reward distributors to the final two gauges.

The match between these.....

Vulnerable code:
+ require(rewardDistributors.length == gauges.length);
  for (uint256 i = 0; i < rewardDistributors.length; i++) {
+     IStrategy oldStrategy = IStrategy(locker.governance());
+     require(oldStrategy.multigauges(gauges[i]) == rewardDistributors[i]);
      ...
  }
LOW severity: Optimizer rounds negative feeDiff values to zero. In the optimal deposit formula in Optimizer.sol, we calculate the `feeDiff` between Convex and StakeDAO. `feeDiff` represents how much more profitable each dollar earned in Convex is than StakeDAO, taking into account fees and incentives.

Since StakeDAO's fee is 16% and Convex is 17%, and we are...

Vulnerable code:
uint256 feeDiff = boost + stakeDaoTotalFee > convexTotalFee ? stakeDaoTotalFee + boost - convexTotalFee : 0;
LOW severity: Fallback & vault fees must remain at zero for Optimizer to work as intended. In the OnlyBoost whitepaper, it's explained that fees on Convex rewards must be set to zero in order to justify the optimizations provided by the protocol.

> As we said earlier, it is a very nice increase but doesn't justify an additional performance fee on top of Convex, that's why it's...

Vulnerable code:
protocolFees = _chargeProtocolFees(rewardTokenAmount);

/// @notice Internal function to charge protocol fees from `rewardToken` claimed by the locker.
function _chargeProtocolFees(uint256 _amount) internal view returns (uint256 _feeAccrued) {
    if (_amount == 0) return 0;

    uint256 protocolFeesPercent = factory().protocolFeesPercent();
    if (protocolFeesPercent == 0) return 0;

    _feeAccrued = _amount.mulDiv(protocolFeesPercent, DENOMINATOR);
}
LOW severity: `setGauge()` and `setRewardDistributor()` should revoke old approvals. When `setGauge()` or `setRewardDistributor()` is called, we set the local mappings, and then approve the corresponding token for transfers.

In `setGauge()`, we approve the Curve gauge to spend the LP token from the locker:

In `setRewardDistributor()`, we approve the reward distributor to spend...

Vulnerable code:
function setGauge(address token, address gauge) external onlyGovernanceOrFactory {
    if (token == address(0)) revert ADDRESS_NULL();
    if (gauge == address(0)) revert ADDRESS_NULL();

    gauges[token] = gauge;

    /// Approve trough the locker.
    locker.safeExecute(token, 0, abi.encodeWithSignature("approve(address,uint256)", gauge, 0));
    locker.safeExecute(token, 0, abi.encodeWithSignature("approve(address,uint256)", gauge, type(uint256).max));
}

function setRewardDistributor(address gauge, address rewardDistributor) external onlyGovernanceOrFactory {
    if (gauge == address(0) || rewardDistributor == address(0)) revert ADDRESS_NULL();
    rewardDistributors[gauge] = rewardDistributor;

    /// Approve the rewardDistributor to spend token.
    SafeTransferLib.safeApproveWithRetry(rewardToken, rewardDistributor, type(uint256).max);
}

function setGauge(address token, address gauge) external onlyGovernanceOrFactory {
    if (token == address(0)) revert ADDRESS_NULL();
    if (gauge == address(0)) revert ADDRESS_NULL();

+   address oldGauge = gauges[token];
+   if (oldGauge != address(0)) {
+     locker.safeExecute(token, 0, abi.encodeWithSignature("approve(address,uint256)", oldGauge, 0));
+   }

    gauges[token] = gauge;

    /// Approve trough the locker.
    locker.safeExecute(token, 0, abi.encodeWithSignature("approve(address,uint256)", gauge, 0));
    locker.safeExecute(token, 0, abi.encodeWithSignature("approve(address,uint256)", gauge, type(uint256).max));
}
LOW severity: `_isValidToken()` check should not allow SDT. When a new vault and reward distributor are created using the PoolFactory, we add the reward tokens to the new StakeDAO reard distributor as follows:
1) We manually add CRV and SDT as reward tokens.
2) We iterate through all extra reward tokens from the Curve gauge, and if they pass the...

Vulnerable code:
if (_isValidToken(_extraRewardToken)) {
    /// Then we add the extra reward token to the reward distributor through the strategy.
    strategy.addRewardToken(_gauge, _extraRewardToken);
}

function _isValidToken(address _token) internal view override returns (bool) {
    /// We can't add the reward token as extra reward.
    /// We can't add special pools like the Ve Funder.
    if (_token == rewardToken || _token == VE_FUNDER) return false;

    /// If the token is available as an inflation receiver, it's not valid.
    try GAUGE_CONTROLLER.gauge_types(_token) {
        return false;
    } catch {
        return true;
    }
}

@external
def add_reward(_reward_token: address, _distributor: address):
    """
    @notice Set the active reward contract
    """
    assert msg.sender == self.admin  # dev: only owner

    reward_count: uint256 = self.reward_count
    assert reward_count < MAX_REWARDS
    assert self.reward_data[_reward_token].distributor == ZERO_ADDRESS

    self.reward_data[_reward_token].distributor = _distributor
    self.reward_tokens[reward_count] = _reward_token
    self.reward_count = reward_count + 1
HIGH severity: ERinterest model miscalculates utilization rate due to reserves. Like the other interest models, `CErc721InterestModel.sol` calculates the utilization rate as follows:

This formula makes sense in other markets, because `reserves` are fees earned by the protocol that will show up in the `cash` value (they are tokens held by the market), but should not be taken...

Vulnerable code:
function utilizationRate(uint cash, uint borrows, uint reserves) public pure returns (uint) {
    // Utilization rate is 0 when there are no borrows
    if (borrows == 0) {
        return 0;
    }

    return borrows * BASE / (cash + borrows - reserves);
}
HIGH severity: High ERutilization rate can be exploited to steal funds. As discussed in [H-02], the utilization rate for ERC721 markets can become extremely high if the `reserves` value becomes large.

If the interest market is ever set up with an 18 decimal token, an attacker can exploit this vulnerability to force liquidate innocent users or to steal funds from the...

Vulnerable code:
uint rateToPool = borrowRate * oneMinusReserveFactor / BASE;
uint supplyRate = utilizationRate(cash, borrows, reserves) * rateToPool / BASE;
MEDIUM severity: Collateral token interest is accrued after relative price calculation in liquidations. When `batchLiquidateBorrow()` is called, we calculate the amount of collateral tokens to seize by taking the amount of liquidatable assets paid back and finding the relative exchange rate between the two. This logic is performed in `liquidateCalculateSeizeTokensNormed()`:

This function sets the...

Vulnerable code:
function liquidateCalculateSeizeTokensNormed(address cTokenCollateral, uint normedRepayAmount) public view returns (uint) {
    uint priceCollateralMantissa = oracle.getUnderlyingPrice(CToken(cTokenCollateral));
    if (priceCollateralMantissa == 0) {
        revert PriceError();
    }

    uint exchangeRateMantissa = CToken(cTokenCollateral).exchangeRateStored(); // Note: reverts on error
    uint numerator = liquidationIncentiveMantissa * normedRepayAmount * expScale;
    uint denominator = priceCollateralMantissa * exchangeRateMantissa;

    uint seizeTokens = numerator / denominator;

    return seizeTokens;
}

uint seizeTokens = liquidateCalculateSeizeTokensNormed(address(cTokenCollaterals[i]), liquidatedValueRemaining);
uint actualSeizeTokens;

uint borrowerBalance = cTokenCollaterals[i].balanceOf(borrower);
if (borrowerBalance < seizeTokens) {
    // can't seize more collateral than owned by the borrower
    actualSeizeTokens = borrowerBalance;
} else {
    actualSeizeTokens = seizeTokens;
}

actualSeizeTokens = cTokenCollaterals[i]._seize(msg.sender, borrower, actualSeizeTokens);
MEDIUM severity: NFT oracle missing Chainlink liveness checks. When `getUnderlyingPrice()` is called on the Chainlink oracle, the recommended liveness checks are performed:

However, in the `FloorPriceFeedAdaptor`, we calculate the NFT's USD value by pulling from two separate oracles and combining the values. When these values are pulled, no liveness checks...

Vulnerable code:
(
    /*uint80 roundID*/,
    int rate,
    /*uint startedAt*/,
    uint updatedAt,
    /*uint80 answeredInRound*/
) = feed.latestRoundData();

...

// check if price is stale
uint stalePriceDelay = stalePriceDelays[asset];
if (stalePriceDelay != 0 && block.timestamp > updatedAt + stalePriceDelay) {
    revert StalePrice();
}

function latestAnswer() public view returns (int256) {
    (
        ,
        int floorRate,
        ,
        ,

    ) = floorPriceFeed.latestRoundData();

    (
        ,
        int ethRate,
        ,
        ,

    ) = ethPriceFeed.latestRoundData();

    return floorRate * ethRate / 1e18; // scale to 8 decimals places like Chainlink
}

function latestRoundData()
    public
    view
    returns (
        uint80 roundId,
        int256 answer,
        uint256 startedAt,
        uint256 updatedAt,
        uint80 answeredInRound
    ) {
    return (0, latestAnswer(), 0, 0, 0);
}
MEDIUM severity: BaseJumpRateModelV2 uses outdated blockPerYear value. In `BaseJumpRateModelV2.sol`, which is intended to be used as the rate model for all non-ERC721 markets, we use the following constant for `blocksPerYear`:

This value is used when setting interest rates, which are inputted on an annual basis and adjusted to the per block rate using the following...

Vulnerable code:
uint public constant blocksPerYear = 2102400;

function updateJumpRateModelInternal(uint baseRatePerYear, uint multiplierPerYear, uint jumpMultiplierPerYear, uint kink_) internal {
    baseRatePerBlock = baseRatePerYear / blocksPerYear;
    multiplierPerBlock = (multiplierPerYear * BASE) / (blocksPerYear * kink_);
    jumpMultiplierPerBlock = jumpMultiplierPerYear / blocksPerYear;
    kink = kink_;

    emit NewInterestParams(baseRatePerBlock, multiplierPerBlock, jumpMultiplierPerBlock, kink);
}

baseRatePerYear = 1e18
baseRatePerBlock = 1e18 / 2102400 = 476190476190476
actualRatePerYear = 476190476190476 * 2628000 = 1.25e18
MEDIUM severity: Liquidated NFTs can earn more than liquidation incentive. The liquidation incentive (currently set to 1.08x) is used to calculate the surplus collateral a liquidator will receive in excess of the amount they repay. It is intended to define the liquidation rewards.

This works by allowing the liquidator to input the tokens they wish to repay, and the...

Vulnerable code:
(err, afterRatio) = getAccountDebtRatio(borrower);
if ((err != uint(Error.NO_ERROR) && err != uint(Error.TOO_LITTLE_INTEREST_RESERVE)) || afterRatio > beforeRatio) {
    revert LiquidateSeizeTooMuch();
}
MEDIUM severity: No check that NFTs aren't being claimed for free in `topUpInterestShortfall()`. In the `topUpInterestShortfall()` function, we allow a liquidator to top up the interest balance of a borrower, in exchange for an equal amount of any of their collateral tokens.

Because NFTs are only liquidated or traded in round increments, the following logic ensures the amount used to top up...

Vulnerable code:
if (CTokenInterface(cTokenCollateral).marketType() == 2) {
    // nft market
    uint oneNFTAmount = doubleScale / exchangeRateCollateralMantissa;
    if (actualSeizeTokens % oneNFTAmount != 0) {
        // ensure whole nft seize size by rounding up to the next whole NFT
        actualSeizeTokens = ((actualSeizeTokens / oneNFTAmount) + 1) * oneNFTAmount;
    }
}

uint borrowerBalance = CTokenInterface(cTokenCollateral).balanceOf(borrower);
if (borrowerBalance < actualSeizeTokens) {
    actualSeizeTokens = borrowerBalance;
}

if (actualSeizeTokens != seizeTokens) {
    actualTopUpTokens = actualTopUpTokens * actualSeizeTokens / seizeTokens;
}

uint actualTopUpTokens = shortfallUsd * doubleScale / (priceInterestMantissa * exchangeRateInterestMantissa);

if (actualTopUpTokens == 0 && actualSeizeTokens > 0) {
    revert TopUpZero();
}
MEDIUM severity: Some NFTs may be incompatible with pools due to `totalSupply()` check. When initializing a new ERC721 market, we end the `initialize()` function with a `totalSupply()` call to the underlying NFT.

This is forked from a check that Compound performs on their `CErc20` markets, which is intended to ensure that the underlying token is a valid ERC20. This check makes sense.....

Vulnerable code:
function initialize(address underlying_,
    ComptrollerInterface comptroller_,
    InterestRateModel interestRateModel_,
    uint initialExchangeRateMantissa_,
    string memory name_,
    string memory symbol_,
    uint8 decimals_
) public {

    ...

    // Set underlying and sanity check it
    underlying = underlying_;
    EIP20Interface(underlying).totalSupply();
}
MEDIUM severity: All payable calls will revert when sent to CEtherDelegator. The Delegator contracts are intended to pass along any calls to their corresponding implementations.

Some delegators perform this logic by manually implementing each underlying function. Others (such as `CEtherDelegator.sol`) simply use a fallback function which is intended to capture all...

Vulnerable code:
fallback() external payable {
    if (msg.value > 0) {
        revert CannotReceiveValueGtZero();
    }

    ...
}
MEDIUM severity: `beforeRatio` taken before interest accrual in liquidations. In `batchLiquidateBorrow()`, the fundamental check on the liquidation is that the account debt ratio after is less than or equal to what it was before.

Along the pathway to liquidation, all relevant markets have their interest accrued (either in the `_liquidateBorrow()` function or the `_seize()`.....

Vulnerable code:
if ((err != uint(Error.NO_ERROR) && err != uint(Error.TOO_LITTLE_INTEREST_RESERVE)) || afterRatio > beforeRatio) {
    revert LiquidateSeizeTooMuch();
}
MEDIUM severity: Uncollected interest on NFTs not counted in liquidity calculation. When `getHypotheticalAccountLiquidityInternal()` is called to calculate whether a user's overall account is in good standing or has a shortfall, it calculates the value of their collateral (discounted by the collateral factor) and borrows to compare.

In order to ensure we are accounting for the...

Vulnerable code:
(oErr, vars.cTokenBalance, vars.borrowBalance, vars.exchangeRateMantissa, vars.interestBalance) = asset.getAccountSnapshot(account);

vars.sumInterestOwed = vars.sumInterestOwed + vars.interestBalance * interestOraclePriceMantissa / 1e18;

vars.sumBorrowPlusEffects = vars.sumBorrowPlusEffects + vars.sumInterestOwed;
LOW severity: Interest market's `initialize()` function is skipped due to incorrect signature. Both `CErc20InterestMarketDelegator.sol` and `CErc20InterestMarketNoProxy.sol` call the underlying implementation's `initialize()` function in their constructor.

Looking at the no proxy version as an example, we see:

We can see that this funciton call has 8 arguments, ending in a `3`, which...

Vulnerable code:
initialize(
    underlying_,
    comptroller_,
    interestRateModel_,
    initialExchangeRateMantissa_,
    name_,
    symbol_,
    decimals_,
    3
);

function initialize(address underlying_,
    ComptrollerInterface comptroller_,
    InterestRateModel interestRateModel_,
    uint initialExchangeRateMantissa_,
    string memory name_,
    string memory symbol_,
    uint8 decimals_
) public {
    super.initialize(underlying_, comptroller_, interestRateModel_, initialExchangeRateMantissa_, name_, symbol_, decimals_, 3);
}
LOW severity: `getAccountDebtRatio()` behaves unexpectedly. The Comptroller's `getAccountDebtRatio()` is a public view function that can be called by external contracts to determine the debt ratio of a given account.

The function is defined as follows:

In the case where the account is in a shortfall, the function returns `borrows * 1e18 / collateral`,...

Vulnerable code:
function getAccountDebtRatio(address account) public view returns (uint, uint) {
    // uint[4] memory retVals; /* liquidity, shortfall, sumCollateral, sumBorrowPlusEffects */
    (Error err, uint[4] memory retVals) = getHypotheticalAccountLiquidityInternal(account, CToken(address(0)), 0, 0);
    if (retVals[1] == 0) {
        return (uint(err), 0);
    } else if (retVals[2] == 0) {
        return (uint(err), type(uint).max);
    }
    return (uint(err), retVals[3] * 1e18 / retVals[2]);
}
LOW severity: Underlying oracle decimals should be confirmed by `FloorPriceFeedAdaptor`. To calculate the value of the NFT in USD with 8 decimals of precision, the `FloorPriceFeedAdaptor` performs the following:

This takes the price of the NFT in ETH, multiplies it by the ETH price in USD, and divides by 1e18.

This should return 8 decimals because the `floorRate` will be in 18...

Vulnerable code:
constructor(AggregatorV3Interface floorPriceFeed_, AggregatorV3Interface ethPriceFeed_) public {
+   require(floorPriceFeed_.decimals() == 18, "FloorPriceFeedAdaptor: floorPriceFeed decimals must be 18");
+   require(ethPriceFeed_.decimals() == 8, "FloorPriceFeedAdaptor: ethPriceFeed decimals must be 8");
    floorPriceFeed = floorPriceFeed_;
    ethPriceFeed = ethPriceFeed_;
}