Farseen0/scar-pairs huggingface dataset:
HIGH severity: `yUSDeVault` edge cases should be explicitly handled to prevent view functions from reverting. **Description:** Per the ERC-4626 specification, the preview functions "MUST NOT revert due to vault specific user/global limits. MAY revert due to other conditions that would also cause mint/deposit/redeem/withdraw to revert".

As noted using `// @audit` tags in the code snippets above,...

Vulnerable code:
function totalAccruedUSDe() public view returns (uint256) {
@>      uint pUSDeAssets = super.totalAssets();  // @audit - should return early if pUSDeAssets is zero to avoid reverting in the call below

@>      uint USDeAssets = _convertAssetsToUSDe(pUSDeAssets, true);
        return USDeAssets;
    }

    function _convertAssetsToUSDe (uint pUSDeAssets, bool withYield) internal view returns (uint256) {
@>      uint sUSDeAssets = pUSDeVault.previewRedeem(withYield ? address(this) : address(0), pUSDeAssets); // @audit - this can revert if passing yUSDe as the caller when it has no pUSDe balance
        uint USDeAssets = sUSDe.previewRedeem(sUSDeAssets);
        return USDeAssets;
    }

    function previewDeposit(uint256 pUSDeAssets) public view override returns (uint256) {
        uint underlyingUSDe = _convertAssetsToUSDe(pUSDeAssets, false);

@>      uint yUSDeShares = _valueMulDiv(underlyingUSDe, totalAssets(), totalAccruedUSDe(), Math.Rounding.Floor); // @audit - should explicitly handle the case where totalAccruedUSDe() returns zero rather than relying on _valueMulDiv() behaviour
        return yUSDeShares;
    }

    function previewMint(uint256 yUSDeShares) public view override returns (uint256) {
@>      uint underlyingUSDe = _valueMulDiv(yUSDeShares, totalAccruedUSDe(), totalAssets(), Math.Rounding.Ceil); // @audit - should explicitly handle the case where totalAccruedUSDe() and/or totalAssets() returns zero rather than relying on _valueMulDiv() behaviour
        uint pUSDeAssets = pUSDeVault.previewDeposit(underlyingUSDe);
        return pUSDeAssets;
    }

    function _valueMulDiv(uint256 value, uint256 mulValue, uint256 divValue, Math.Rounding rounding) internal view virtual returns (uint256) {
        return value.mulDiv(mulValue + 1, divValue + 1, rounding);
    }

function previewYield(address caller, uint256 shares) public view virtual returns (uint256) {
        if (PreDepositPhase.YieldPhase == currentPhase && caller == address(yUSDe)) {

            uint total_sUSDe = sUSDe.balanceOf(address(this));
            uint total_USDe = sUSDe.previewRedeem(total_sUSDe);

            uint total_yield_USDe = total_USDe - Math.min(total_USDe, depositedBase);

@>          uint y_pUSDeShares = balanceOf(caller); // @audit - should return early if this is zero to avoid reverting below
@>          uint caller_yield_USDe = total_yield_USDe.mulDiv(shares, y_pUSDeShares, Math.Rounding.Floor);

            return caller_yield_USDe;
        }
        return 0;
    }

    function previewRedeem(address caller, uint256 shares) public view virtual returns (uint256) {
        return previewRedeem(shares) + previewYield(caller, shares);
    }
HIGH severity: Use explicit sizes instead of `uint`. **Description:** While `uint` defaults to `uint256`, it is considered good practice to use the explicit types including the size and to avoid using `uint`:

**Strata:** Fixed in commit [61f5910](https://github.com/Strata-Money/contracts/commit/61f591088754e2666355307cf1e11e6440af8572).

**Cyfrin:**....

Vulnerable code:
predeposit/yUSDeDepositor.sol
65:        uint beforeAmount = asset.balanceOf(address(this));
73:        uint pUSDeShares = pUSDeDepositor.deposit(asset, amount, address(this));

predeposit/MetaVault.sol
53:        uint baseAssets = IERC4626(token).previewRedeem(tokenAssets);
54:        uint shares = previewDeposit(baseAssets);
70:        uint baseAssets = previewMint(shares);
71:        uint tokenAssets = IERC4626(token).previewWithdraw(baseAssets);
211:        uint balance = IERC20(vaultAddress).balanceOf(address(this));
219:        uint length = assetsArr.length;
220:        for (uint i = 0; i < length; i++) {
240:    function redeemRequiredBaseAssets (uint baseTokens) internal {
241:        for (uint i = 0; i < assetsArr.length; i++) {
243:            uint totalBaseTokens = vault.previewRedeem(vault.balanceOf(address(this)));

predeposit/pUSDeVault.sol
62:            uint total_sUSDe = sUSDe.balanceOf(address(this));
63:            uint total_USDe = sUSDe.previewRedeem(total_sUSDe);
65:            uint total_yield_USDe = total_USDe - Math.min(total_USDe, depositedBase);
67:            uint y_pUSDeShares = balanceOf(caller);
68:            uint caller_yield_USDe = total_yield_USDe.mulDiv(shares, y_pUSDeShares, Math.Rounding.Floor);
121:            uint sUSDeAssets = sUSDe.previewWithdraw(assets);
138:        uint USDeBalance = USDe.balanceOf(address(this));
171:        uint USDeBalance = USDe.balanceOf(address(this));

predeposit/yUSDeVault.sol
38:        uint pUSDeAssets = super.totalAssets();
39:        uint USDeAssets = _convertAssetsToUSDe(pUSDeAssets, true);
43:    function _convertAssetsToUSDe (uint pUSDeAssets, bool withYield) internal view returns (uint256) {
44:        uint sUSDeAssets = pUSDeVault.previewRedeem(withYield ? address(this) : address(0), pUSDeAssets);
45:        uint USDeAssets = sUSDe.previewRedeem(sUSDeAssets);
59:        uint underlyingUSDe = _convertAssetsToUSDe(pUSDeAssets, false);
60:        uint yUSDeShares = _valueMulDiv(underlyingUSDe, totalAssets(), totalAccruedUSDe(), Math.Rounding.Floor);
74:        uint underlyingUSDe = _valueMulDiv(yUSDeShares, totalAccruedUSDe(), totalAssets(), Math.Rounding.Ceil);
75:        uint pUSDeAssets = pUSDeVault.previewDeposit(underlyingUSDe);
HIGH severity: Prefix internal and private function names with `_` character. **Description:** It is considered good practice in Solidity to prefix internal and private function names with `_` character. This is done sometimes but not other times; ideally apply this consistently:

**Strata:** Fixed in commit...

Vulnerable code:
predeposit/PreDepositPhaser.sol
15:    function setYieldPhaseInner () internal {

predeposit/yUSDeDepositor.sol
54:    function deposit_pUSDe (address from, uint256 amount, address receiver) internal returns (uint256) {
62:    function deposit_pUSDeDepositor (address from, IERC20 asset, uint256 amount, address receiver) internal returns (uint256) {

predeposit/PreDepositVault.sol
59:    function onAfterDepositChecks () internal view {
64:    function onAfterWithdrawalChecks () internal view {

predeposit/pUSDeVault.sol
93:    function _deposit(address caller, address receiver, uint256 assets, uint256 shares) internal override {
115:    function _withdraw(address caller, address receiver, address owner, uint256 assets, uint256 shares) internal override {
177:    function stakeUSDe(uint256 USDeAssets) internal returns (uint256) {

predeposit/yUSDeVault.sol
43:    function _convertAssetsToUSDe (uint pUSDeAssets, bool withYield) internal view returns (uint256) {
79:    function _deposit(address caller, address receiver, uint256 pUSDeAssets, uint256 shares) internal override {
86:    function _withdraw(address caller, address receiver, address owner, uint256 pUSDeAssets, uint256 shares) internal override {
101:    function _valueMulDiv(uint256 value, uint256 mulValue, uint256 divValue, Math.Rounding rounding) internal view virtual returns (uint256) {

predeposit/MetaVault.sol
84:    function _deposit(address token, address caller, address receiver, uint256 baseAssets, uint256 tokenAssets, uint256 shares) internal virtual {
160:    ) internal virtual {
175:    function requireSupportedVault(address token) internal view {
191:    function addVaultInner (address vaultAddress) internal {
209:    function removeVaultAndRedeemInner (address vaultAddress) internal {
231:    function redeemMetaVaults () internal {
240:    function redeemRequiredBaseAssets (uint baseTokens) internal {

predeposit/pUSDeDepositor.sol
92:    function deposit_sUSDe (address from, uint256 amount, address receiver) internal returns (uint256) {
102:    function deposit_USDe (address from, uint256 amount, address receiver) internal returns (uint256) {
114:    function deposit_viaSwap (address from, IERC20 token, uint256 amount, address receiver) internal returns (uint256) {
146:    function getPhase () internal view returns (PreDepositPhase phase) {

test/ethena/StakedUSDe.sol
190:  function _checkMinShares() internal view {
203:    internal
225:    internal
239:  function _updateVestingAmount(uint256 newVestingAmount) internal {
251:  function _beforeTokenTransfer(address from, address to, uint256) internal virtual {

test/ethena/SingleAdminAccessControl.sol
72:  function _grantRole(bytes32 role, address account) internal override returns (bool) {
HIGH severity: Superfluous vault support validation can be removed from `pUSDeDepositor::deposit`. **Description:** If the caller to `pUSDeDepositor::deposit` attempts to deposit a vault token that is not `USDe` or one of those preconfigured with an auto swap path, it will first query `MetaVault::isAssetSupported`:

If the specified vault token fails all validation then it falls through to the...

Vulnerable code:
function deposit(IERC20 asset, uint256 amount, address receiver) external returns (uint256) {
        address user = _msgSender();
        ...
        IMetaVault vault = IMetaVault(address(pUSDe));
@>      if (vault.isAssetSupported(address(asset))) {
            SafeERC20.safeTransferFrom(asset, user, address(this), amount);
            asset.approve(address(vault), amount);
            return vault.deposit(address(asset), amount, receiver);
        }
@>      revert InvalidAsset(address(asset));
    }

function deposit(address token, uint256 tokenAssets, address receiver) public virtual returns (uint256) {
        if (token == asset()) {
            return deposit(tokenAssets, receiver);
        }
@>      requireSupportedVault(token);
        ...
    }

    function requireSupportedVault(address token) internal view {
        address vaultAddress = assetsMap[token].asset;
        if (vaultAddress == address(0)) {
@>          revert UnsupportedAsset(token);
        }
    }

function deposit(IERC20 asset, uint256 amount, address receiver) external returns (uint256) {
        address user = _msgSender();
        ...
        IMetaVault vault = IMetaVault(address(pUSDe));
--      if (vault.isAssetSupported(address(asset))) {
            SafeERC20.safeTransferFrom(asset, user, address(this), amount);
            asset.approve(address(vault), amount);
            return vault.deposit(address(asset), amount, receiver);
--      }
--      revert InvalidAsset(address(asset));
    }
HIGH severity: Unnecessarily complex iteration logic in `MetaVault::redeemMetaVaults` can be simplified. **Description:** `MetaVault::redeemMetaVaults` is currently implemented as a while loop, indexing the first array element and calling `MetaVault::removeVaultAndRedeemInner` which implements a "replace-and-pop" solution for removing elements from the `assetsArr` array:

While this logic is still...

Vulnerable code:
function removeVaultAndRedeemInner (address vaultAddress) internal {
        // Redeem
        uint balance = IERC20(vaultAddress).balanceOf(address(this));
        if (balance > 0) {
            IERC4626(vaultAddress).redeem(balance, address(this), address(this));
        }

        // Clean
        TAsset memory emptyAsset;
        assetsMap[vaultAddress] = emptyAsset;
        uint length = assetsArr.length;
        for (uint i = 0; i < length; i++) {
            if (assetsArr[i].asset == vaultAddress) {
@>              assetsArr[i] = assetsArr[length - 1];
@>              assetsArr.pop();
                break;
            }
        }
    }

    function redeemMetaVaults () internal {
        while (assetsArr.length > 0) {
@>          removeVaultAndRedeemInner(assetsArr[0].asset);
        }
    }
HIGH severity: Lack of validation when updating prizes can lead to `lotAmount` underflow when randomness is fulfilled. **Description:** In [`SpinGame::_fulfillRandomness`](https://github.com/Consensys/linea-hub/blob/0af327319636960e9683897c5935aa1a78d1ded5/contracts/src/Spin.sol#L595-L603), there's an `unchecked` block that decrements `prize.lotAmount` without validating its current value:

However, the comment...

Vulnerable code:
/// Should never underflow due to earlier check.
unchecked {
    prize.lotAmount -= 1;
}

if (prize.lotAmount == 0) {
    totalProbabilities -= prizeProbability;
    prize.probability = 0;
}

function testFulfillRandomnessWith0LotAmount() external {
    MockERC20 token = new MockERC20("Test Token", "TST");
    ISpinGame.Prize[] memory prizesToUpdate = new ISpinGame.Prize[](1);
    uint256[] memory empty = new uint256[](0);

    prizesToUpdate[0] = ISpinGame.Prize({
        tokenAddress: address(token),
        amount: 500 * 1e18,
        lotAmount: 0,
        probability: 1e8,
        availableERC721Ids: empty
    });

    vm.prank(controller);
    spinGame.updatePrizes(prizesToUpdate);

    uint64 nonce = 1;
    uint64 expirationTimestamp = uint64(block.timestamp + 1);
    uint64 boost = 1e8;

    ISpinGame.ParticipationRequest memory request = ISpinGame.ParticipationRequest({
        user: user,
        expirationTimestamp: expirationTimestamp,
        nonce: nonce,
        boost: boost
    });

    bytes32 messageHash = spinGame.hashParticipationExt(request);
    (uint8 v, bytes32 r, bytes32 s) = vm.sign(signer, messageHash);
    ISpinGame.Signature memory signature = ISpinGame.Signature(r, s, v);

    assertFalse(spinGame.nonces(user, nonce));

    vm.prank(user);
    uint256 requestId = spinGame.participate(nonce, expirationTimestamp, boost, signature);
    assertTrue(spinGame.nonces(user, nonce));

    bytes memory extraData = new bytes(0);
    bytes memory data = abi.encode(requestId, extraData);

    uint256 round = 15608646;
    bytes memory dataWithRound = abi.encode(round, data);

    vm.prank(vrfOperator);
    spinGame.fulfillRandomness(2, dataWithRound);

    uint32[] memory prizeIds = new uint32[](1);
    prizeIds[0] = 0;
    uint256[] memory amounts = spinGame.getUserPrizesWon(user, prizeIds);
    assertEq(amounts[0], 1);
    assertEq(spinGame.hasWonPrize(user, 0), true);

    ISpinGame.Prize memory prize = spinGame.getPrize(0);
    assertEq(prize.lotAmount,type(uint32).max);
}

uint32 lotAmount = _prizes[i].lotAmount;

+   if (lotAmount == 0) {
+       revert InvalidLotAmount();
+   }

    if (prizeAmount == 0) {
        if (erc721IdsLen != lotAmount) {
            revert MismatchERC721PrizeAmount(erc721IdsLen, lotAmount);
        }
    } else {
        if (erc721IdsLen != 0) {
            revert ERC20PrizeWrongParam();
        }
    }
LOW severity: Asynchronous VRF request fulfillment uses stale or incorrect parameters due to lack of request-specific data tracking. **Description:** The `SpinGame` contract has a critical architectural flaw in how it handles asynchronous VRF requests. The contract stores user boost values and prize configurations globally rather than tying them to specific requests, leading to inconsistent game behavior when prize structures...

Vulnerable code:
// Scenario 1: Prize structure changes between request and fulfillment
1. User calls participate() when Prize A (50% chance) and Prize B (30% chance) are available
2. Controller calls updatePrizes() with Prize C (60% chance) and Prize D (20% chance)
3. VRF fulfills the request using new prize structure with C and D instead of A and B

// Scenario 2: Multiple participations with different boosts
1. User calls participate() with 150% boost, gets requestId1
2. User calls participate() with 500% boost, gets requestId2
3. VRF fulfills requestId1 but uses 500% boost instead of 150%

+ uint256 public prizeVersion;
+ mapping(uint256 requestId => uint64 boost) public requestIdToBoost;
+ mapping(uint256 requestId => uint256 prizeVersion) public requestIdToPrizeVersion;

function updatePrizes(Prize[] calldata _prizes) external onlyController {
    delete prizeIds;
    totalProbabilities = 0;
+   prizeVersion++;
    _addPrizes(_prizes);
}

function participate(
    uint64 _nonce,
    uint256 _expirationTimestamp,
    uint64 _boost,
    Signature calldata _signature
) external returns (uint256) {
    // ... existing validation ...

-   userToBoost[user] = _boost;
    uint256 requestId = _requestRandomness("");

    requestIdToUser[requestId] = user;
    requestIdTimestamp[requestId] = block.timestamp;
+   requestIdToBoost[requestId] = _boost;
+   requestIdToPrizeVersion[requestId] = prizeVersion;

    // ... rest of function ...
}

function _fulfillRandomness(
    uint256 _randomness,
    uint256 _requestId,
    bytes memory
) internal override {
    address user = requestIdToUser[_requestId];
    if (user == address(0)) {
        revert InvalidRequestId(_requestId);
    }

+   // Verify prize version hasn't changed
+   if (requestIdToPrizeVersion[_requestId] != prizeVersion) {
+       revert PrizeVersionMismatch();
+   }

-   uint64 userBoost = userToBoost[user];
+   uint64 userBoost = requestIdToBoost[_requestId];

    // ... rest of fulfillment logic ...

+   delete requestIdToBoost[_requestId];
+   delete requestIdToPrizeVersion[_requestId];
}
HIGH severity: Stale request id mapping persists for losing spins. **Description:** One change from the previous version of `SpinGame` is the removal of the "LuckyNFT," which served as a consolation prize for users who did not win a "proper" prize.

As part of this change, the clearing of the request mapping (`requestIdToUser`) was moved so that it now occurs...

Vulnerable code:
userToPrizesWon[user][selectedPrizeId] += 1;
            delete requestIdToUser[_requestId];
            emit PrizeWon(user, selectedPrizeId);

            return;
        }
    }
    emit NoPrizeWon(user);
}

function _fulfillRandomness(
    uint256 _randomness,
    uint256 _requestId,
    bytes memory
) internal override {
    address user = requestIdToUser[_requestId];
    if (user == address(0)) {
        revert InvalidRequestId(_requestId);
    }
+   delete requestIdToUser[_requestId];

    ...

            userToPrizesWon[user][selectedPrizeId] += 1;
-           delete requestIdToUser[_requestId];
            emit PrizeWon(user, selectedPrizeId);


            return;
        }
    }
    emit NoPrizeWon(user);
}
HIGH severity: Cache `prize.amount` in `SpinGame::_transferPrize`. **Description:** In `SpinGame::_transferPrize`, when transferring either an ERC20 or native token prize, the `prize.amount` field is read three times: once for the `if` check, once to compare against the contract balance, and again when executing the transfer:

This results in three separate reads.....

Vulnerable code:
if (prize.amount > 0) {
    ...
    if (contractBalance < prize.amount) {
        revert PrizeAmountExceedsBalance(..., prize.amount, contractBalance);
    }
    ...
    token.safeTransfer(_winner, prize.amount);
}

uint256 amount = prize.amount;
if (amount > 0) {
    ...
    if (contractBalance < amount) {
        revert PrizeAmountExceedsBalance(..., amount, contractBalance);
    }
    ...
    token.safeTransfer(_winner, amount);
}
HIGH severity: `PledgeManager::pledge`, `refundTokens` will revert due to overflow when `pricePerToken * numTokens > type(uint32).max`. **Description:** `PledgeManager::pledge` multiplies two `uint32` variables and stores the result into a `uint256`, attempting to account for when the multiplication returns a value greater than `type(uint32).max`:

`PledgeManager::refundTokens` does the same thing:

However this won't work...

Vulnerable code:
uint256 stablecoinAmount = pricePerToken * numTokens; // account for overflow

uint256 refundAmount = numTokens * pricePerToken; //TOOD: overflow check

$ chisel
Welcome to Chisel! Type `!help` to show available commands.
➜ uint32 a = type(uint32).max;
➜ uint32 b = 10;
➜ uint256 c = a * b;
Traces:
  [401] 0xBd770416a3345F91E4B34576cb804a576fa48EB1::run()
    └─ ← [Revert] panic: arithmetic underflow or overflow (0x11)

Error: Failed to execute REPL contract!
HIGH severity: Distribution of payouts will revert due to overflow when payment is made using a stablecoin with high decimals. **Description:** Payouts are meant to be paid using a stablecoin, originally a stablecoin with 6 decimals (USDC). But the system has the capability of changing the stablecoin that is used for payments. Could be USDT (8 decimals), USDS (18 decimals).

As part of the changes made to introduce the...

Vulnerable code:
➜ bool a = type(uint64).max > 20e18;
➜ a
Type: bool
└ Value: false

function payoutBalance(address holder) public returns (uint256) {
        ...
        for (uint16 i = payRangeStart; i >= payRangeEnd; --i) {
            ...

            PayoutInfo memory pInfo = $._payouts[i];
//@audit => `pInfo.amount` set using a stablecoin with high decimals will bring up the payoutAmount beyond the limit of what can fit in a uint64
            payoutAmount +=
                (curEntry.tokenBalance * pInfo.amount) /
                pInfo.totalSupply;
            if (i == 0) break; // to prevent potential overflow
        }
        ...
        if (payoutForwardAddr == address(0)) {
//@audit-issue => overflow will blow up the tx
            holderStatus.calculatedPayout += SafeCast.toUint64(payoutAmount);
        } else {
//@audit-issue => overflow will blow up the tx
            $._holderStatus[payoutForwardAddr].calculatedPayout += SafeCast
                .toUint64(payoutAmount);
        }
HIGH severity: Attacker can make pledge on behalf of users if those users have approved `PledgeManager` to spend their tokens. **Description:** `PledgeManager` requires users to approve it to spend their tokens in order to make pledges. Users can do this by either:
1) using `IERC20Permit::permit` which enforces a nonce for the signer, deadline and domain separator
2) manually by calling `IERC20::approve`

If users use the.....

Vulnerable code:
if (data.usePermit) {
            IERC20Permit(stablecoin).permit(
                signer,
                address(this),
                finalStablecoinAmount,
                block.timestamp + 300,
                data.permitV,
                data.permitR,
                data.permitS
            );
        }
+       else if(msg.sender != signer) revert MsgSenderNotSigner();
HIGH severity: `PaymentSettler` can change `stablecoin` but `RemoraToken` can't resulting in corrupted state with DoS for core functions. **Description:** `RemoraToken` has a `stablecoin` member with a comment that indicates it must match `PaymentSettler`:

But in the updated code there is no way to update `RemoraToken::stablecoin`; previously `DividendManager` which `RemoraToken` inherits from had a `changeStablecoin` function but...

Vulnerable code:
address public stablecoin; //make sure same stablecoin is used here that is used in payment settler

address public stablecoin;

function changeStablecoin(address newStablecoin) external restricted {
    if (newStablecoin == address(0)) revert InvalidAddress();
    stablecoin = newStablecoin;
}

function test_changeStablecoin_inconsistentState() external {
    address newStableCoin = address(new Stablecoin("USDC", "USDC", 0, 6));

    // change stablecoin on PaymentSettler
    paySettlerProxy.changeStablecoin(newStableCoin);
    assertEq(paySettlerProxy.stablecoin(), newStableCoin);

    // now inconsistent with RemoraToken
    assertEq(remoraTokenProxy.stablecoin(), address(stableCoin));
    assertNotEq(paySettlerProxy.stablecoin(), remoraTokenProxy.stablecoin());

    // no way to update RemoraToken::stablecoin
}