Farseen0/scar-pairs huggingface dataset:
LOW severity: `SecuritizeAmmNavProvider` trades when pool price and anchor price differ can leak value. **Description:** Due to the asymmetric smoothing formula and integer truncation, a BUY followed by an immediate SELL at the same `anchorPriceWad` and `marketStatus = CLOSED_MARKET` can return more quote than initially spent (profitable round-trip), leaking value from the virtual pool.

**Impact:**.....

Vulnerable code:
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.22;

import "forge-std/Test.sol";
import {SecuritizeAmmNavProvider} from "../contracts/nav/SecuritizeAmmNavProvider.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";

contract AssetMock {
    uint8 public decimals = 6;
}

contract RoundTripTest is Test {
    SecuritizeAmmNavProvider public navProvider;
    uint256 constant WAD = 1e18;

    function setUp() public {
        AssetMock asset = new AssetMock();
        SecuritizeAmmNavProvider impl = new SecuritizeAmmNavProvider();

        ERC1967Proxy proxy = new ERC1967Proxy(
            address(impl),
            abi.encodeWithSelector(
                SecuritizeAmmNavProvider.initialize.selector,
                100_000e6,  // baseReserves
                100_000e6,  // quoteReserves
                address(asset)
            )
        );

        navProvider = SecuritizeAmmNavProvider(address(proxy));
        navProvider.grantRole(navProvider.EXECUTOR_ROLE(), address(this));
    }

    function test_roundTrip() public {
        uint256 quoteIn = 10e6;
        uint256 anchorPrice = 1.01e18;
        uint8 closedMarket = navProvider.CLOSED_MARKET();

        (uint256 baseOut,) = navProvider.executeBuyBase(quoteIn, anchorPrice, closedMarket);
        (uint256 quoteBack,) = navProvider.executeSellBase(baseOut, anchorPrice, closedMarket);

        assertLt(quoteBack, quoteIn, "Round-trip should result in loss");
    }
}
HIGH severity: Use `SafeER` approval and transfer functions instead of standard IERfunctions for `liquidityToken`. **Description:** The on-ramping and off-ramping processes are linked to external liquidity tokens such as stablecoins whose code is not controlled by the protocol; hence use [`SafeERC20::forceApprove, transfer,...

Vulnerable code:
on-ramp/provider/AllowanceAssetProvider.sol
98:        asset.transferFrom(assetProviderWallet, _buyer, _amount);

on-ramp/BaseOnRamp.sol
122:        liquidityToken.transferFrom(from, address(this), amount);
125:            liquidityToken.transfer(feeManager.feeCollector(), fee);
131:            liquidityToken.approve(address(USDCBridge), amountExcludingFee);
134:            liquidityToken.transfer(custodianWallet, amountExcludingFee);

off-ramp/provider/AllowanceLiquidityProvider.sol
141:        liquidityToken.transferFrom(liquidityProviderWallet, _redeemer, _liquidityAmount);

off-ramp/provider/CollateralLiquidityProvider.sol
186:        collateralToken.transferFrom(collateralProvider, address(this), collateralAmount);
189:        collateralToken.approve(address(externalCollateralRedemption), collateralAmount);
198:        liquidityToken.transfer(_redeemer, amountToSupply);

off-ramp/RedemptionManager.sol
43:            _params.asset.transferFrom(_params.redeemer, _params.liquidityProvider.recipient(), _params.assetAmount);
74:        _params.asset.transferFrom(_params.redeemer, _contractAddress, _params.assetAmount);
80:            _params.asset.transfer(_params.liquidityProvider.recipient(), _params.assetAmount);
96:        _params.liquidityProvider.liquidityToken().transfer(_params.redeemer, userSuppliedAmount);
100:            _params.liquidityProvider.liquidityToken().transfer(IFeeManager(_params.feeManager).feeCollector(), fee);
HIGH severity: In Solidity don't initialize to default values. **Description:** In Solidity don't initialize to default values:

**Securitize:** Fixed in commits [7594671](https://bitbucket.org/securitize_dev/bc-nav-provider-sc/commits/75946718b5129603545c364a2d9c6f57902200d9),...

Vulnerable code:
SecuritizeAmmNavProvider.sol
380:        bool shouldReset = false;

off-ramp/BaseOffRamp.sol
124:        for (uint256 i = 0; i < _countries.length; i++) {
HIGH severity: Cache decimals of underlying asset at initialization in `SecuritizeAmmNavProvider`. **Description:** `SecuritizeAmmNavProvider` has no function to change the value of `asset` after initialization. Since ERC20 tokens typically never changed their decimals, the asset decimals can be cached at initialization to save external calls in `quoteBuyBase, quoteSellBase, executeBuyBase,...

Vulnerable code:
contract SecuritizeAmmNavProvider {
    /* snip : existing storage layout */
+   uint256 public SCALE_DOWN;

    function initialize(uint256 _baseReserves, uint256 _quoteReserves, address _asset) public onlyProxy initializer {
        /* snip : existing code */
        asset = IERC20Metadata(_asset);
+       uint8 d = asset.decimals();
+       require(d <= 18, "decimals > 18");
+       SCALE_DOWN = 10 ** (18 - d);
    }

-       uint8 d = asset.decimals();
-       require(d <= 18, "decimals > 18");
-       uint256 scaleDown = 10 ** (18 - d);
HIGH severity: Active and pending bets can be cancelled by anyone. **Description:** [`Bet::cancel`](https://github.com/gskril/wannabet-v2/blob/d7333369548874cb99e9648ea2424ac1e67cc23f/contracts/src/Bet.sol#L163-L176) allows any address (except the maker) to cancel a bet while it is in the `ACTIVE` or `PENDING` state:

The logic only prevents the maker from...

Vulnerable code:
/// @dev Anybody can cancel an expired bet and send funds back to each party. The maker can cancel a pending bet.
function cancel() external {
    IBet.Bet memory b = _bet;

    // Can't cancel a bet that's already completed
    if (b.status >= IBet.Status.RESOLVED) {
        revert InvalidStatus();
    } else {
        // Pending or active bets at this point
        // The maker can cancel a pending bet, so block them from cancelling an active bet
        // @audit-issue this only prevents `maker` from cancelling, anyone including the taker can cancel an active bet
        if (b.maker == msg.sender && b.status != IBet.Status.PENDING) {
            revert InvalidStatus();
        }
    }

// Anyone can cancel a non-expired ACTIVE bet
function test_ActiveBetCanBeCancelledByAnyone() public {
    // 1. Have the taker accept the bet so it becomes ACTIVE
    vm.startPrank(taker);
    usdc.approve(address(betNoPool), 1000);
    betNoPool.accept();
    vm.stopPrank();

    // Sanity check: bet is ACTIVE
    assertEq(uint(betNoPool.bet().status), uint(IBet.Status.ACTIVE));

    // 2. Warp to a time before resolveBy so the bet is still non-expired
    IBet.Bet memory state = betNoPool.bet();
    vm.warp(uint256(state.resolveBy) - 1);
    assertEq(uint(betNoPool.bet().status), uint(IBet.Status.ACTIVE));

    uint256 makerBalanceBefore = usdc.balanceOf(maker);
    uint256 takerBalanceBefore = usdc.balanceOf(taker);

    // 3. taker cancels the bet
    vm.prank(taker);
    betNoPool.cancel();

    // 4. After cancellation, the bet is marked CANCELLED and funds are refunded
    assertEq(uint(betNoPool.bet().status), uint(IBet.Status.CANCELLED));
    assertEq(usdc.balanceOf(maker) - makerBalanceBefore, 1000);
    assertEq(usdc.balanceOf(taker) - takerBalanceBefore, 1000);
}
HIGH severity: In `Bet::cancel` if one transfer reverts but the other succeeds, one users's tokens are permanently locked in the `Bet` contract. **Description:** `Bet::cancel` does this when refunding token transfers:

But if one of the `transfer` calls reverts but the other succeeds, the transaction still successfully executes. This:
* moves the bet into the `CANCELLED` state
* keeps one user's tokens inside the `Bet`...

Vulnerable code:
try IERC20(b.asset).transfer(b.maker, makerRefund) {} catch {}
try IERC20(b.asset).transfer(b.taker, takerRefund) {} catch {}
MEDIUM severity: Owner of `BetFactory` can subtly rug-pull any caller to `BetFactory::createBet` stealing their tokens. **Description:** Owner of `BetFactory` can rug-pull any caller to `createBet` stealing their tokens by front-running to call `BetFactory::setPool` with the address of a malicious contract that implements the same interface as Aave `supply` function and just transfers tokens to itself.

This works...

Vulnerable code:
// If the pool is set, approve and supply the funds to the pool
if (pool != address(0)) {
    IERC20(initialBet.asset).approve(pool, type(uint256).max);

    _aavePool.supply(
        initialBet.asset,
        initialBet.makerStake,
        address(this),
        0
    );
}
HIGH severity: Use `SafeER` functions instead of standard ERfunctions. **Description:** Use [SafeERC20](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/utils/SafeERC20.sol) functions `safeTransfer`, `safeTransferFrom`, `forceApprove` etc instead of standard ERC20 functions to ensure support for ERC20 tokens with non-standard...

Vulnerable code:
Bet.sol
63:        IERC20(initialBet.asset).transferFrom(
71:            IERC20(initialBet.asset).approve(pool, type(uint256).max);
114:        IERC20(b.asset).transferFrom(msg.sender, address(this), b.takerStake);
150:        IERC20(b.asset).transfer(winner, totalWinnings);
155:            IERC20(b.asset).transfer(_treasury, remainder);
193:        try IERC20(b.asset).transfer(b.maker, makerRefund) {} catch {}
194:        try IERC20(b.asset).transfer(b.taker, takerRefund) {} catch {}
LOW severity: Aave rewards incentives lost as there is no way to claim them. **Description:** When a bet uses an Aave pool, funds are supplied to Aave and can accrue [incentive rewards](https://aave.com/docs/aave-v3/aptos/smart-contracts/incentives), but the contracts provide no way to claim or forward these rewards (no integration with the Aave incentives controller and...

Vulnerable code:
function aave_claimRewards(address reward) external {
    address[] memory assets = new address[](1);
    assets[0] = _bet.asset;
    rewardsController.claimRewards(
        assets,
        type(uint256).max,
        _treasury,
        reward
    );
}
HIGH severity: Use named mappings to explicitly indicate the purpose of keys and values. **Description:** Use named mappings to explicitly indicate the purpose of keys and values:

**WannaBet:** Fixed in commit [a20d1e7](https://github.com/gskril/wannabet-v2/commit/a20d1e7acfbeee00cc891324b022fcf0afdd721b).

**Cyfrin:** Verified.

Vulnerable code:
BetFactory.sol
22:    mapping(address => address) public tokenToPool;
HIGH severity: `BetResolved` event in `Bet::resolve` will have the wrong `totalWinnings` when Aave pool is used. **Description:** In `Bet::resolve`, `BetResolved` is emitted with `totalWinnings = makerStake + takerStake` before checking the actual funds in Aave and capping `totalWinnings` to the contract’s `aTokenBalance`:

When an Aave pool is used (with yield or loss), the event’s `amount` field may not...

Vulnerable code:
uint256 totalWinnings = b.makerStake + b.takerStake;
emit BetResolved(winner, totalWinnings);

// If the funds are in Aave, withdraw them
if (address(_aavePool) != address(0)) {
    uint256 aTokenBalance = IERC20(_aavePool.getReserveAToken(b.asset)).balanceOf(address(this));

    // @audit totalWinnings changed
    totalWinnings = _min(totalWinnings, aTokenBalance);
    _aavePool.withdraw(b.asset, aTokenBalance, address(this));
}
HIGH severity: Taker receives Aave yield for cancelled pending bets. **Description:** If a bet uses an Aave pool but never becomes `ACTIVE` (taker never accepts), only the maker’s stake is supplied to Aave. In `Bet::cancel`, the recovered Aave balance is still split using maker/taker logic:

So the taker can receive part of the maker’s accrued yield (or recovered...

Vulnerable code:
uint256 aTokenBalance = IERC20(_aavePool.getReserveAToken(b.asset))
    .balanceOf(address(this));
_aavePool.withdraw(b.asset, aTokenBalance, address(this));

makerRefund = _min(makerRefund, aTokenBalance);
takerRefund = _min(takerRefund, aTokenBalance - makerRefund);
HIGH severity: `BetFactory::setPool` should validate input pool is legitimate AaveV3 pool and supports input token. **Description:** `BetFactory::setPool` should validate input pool is legitimate AaveV3 pool and supports input token. This can be done by:

1) `BetFactory::constructor` should take as input the address of AaveV3 deployed `PoolAddressesProvider` contract and store it into an immutable variable...

Vulnerable code:
import {IPool} from "@aave-dao/aave-v3-origin/src/contracts/interfaces/IPool.sol";
import {IAToken} from "@aave-dao/aave-v3-origin/src/contracts/interfaces/IAToken.sol";
import {IPoolAddressesProvider} from "@aave-dao/aave-v3-origin/src/contracts/interfaces/IPoolAddressesProvider.sol";

contract BetFactory is Ownable {
    // Base mainnet Aave V3 PoolAddressesProvider
    IPoolAddressesProvider public constant AAVE_ADDRESSES_PROVIDER =
        IPoolAddressesProvider(0xe20fCBdBfFC4Dd138cE8b2E6FBb6CB49777ad64D);

    error InvalidPool();
    error TokenNotSupported();
    error ATokenMismatch();

function setPool(address _token, address _pool) external onlyOwner {
        // Allow setting to zero (disable Aave for this token)
        if (_pool == address(0)) {
            tokenToPool[_token] = address(0);
            emit PoolConfigured(_token, address(0));
            return;
        }

        // Validate against canonical registry
        if (_pool != AAVE_ADDRESSES_PROVIDER.getPool()) {
            revert InvalidPool();
        }

        // Verify token is listed
        address aToken = IPool(_pool).getReserveAToken(_token);
        if (aToken == address(0)) {
            revert TokenNotSupported();
        }

        // Verify bidirectional relationship
        if (IAToken(aToken).UNDERLYING_ASSET_ADDRESS() != _token) {
            revert ATokenMismatch();
        }

        tokenToPool[_token] = _pool;
        emit PoolConfigured(_token, _pool);
    }
HIGH severity: Use `type(uint256).max` when withdrawing from Aave. **Description:** When unwinding Aave positions in `Bet::resolve` and `cancel`, the contract withdraws using the current aToken balance as the `amount` parameter:

Aave’s [recommended pattern](https://aave.com/docs/aave-v3/smart-contracts/pool?utm_source=chatgpt.com#write-methods-withdraw) for...

Vulnerable code:
uint256 aTokenBalance = IERC20(_aavePool.getReserveAToken(b.asset))
    .balanceOf(address(this));
_aavePool.withdraw(b.asset, aTokenBalance, address(this));

uint256 aTokenBalance = _aavePool.withdraw(b.asset, type(uint256).max, address(this));
HIGH severity: Any staker can fully avoid slashing by triggering OOG reverts. **Description:** The finding is similar to [*Malicious actors can force vaults to exit if they wish to get rewards*](#malicious-actors-can-force-vaults-to-exit-if-they-wish-to-get-rewards), but actually different (different fix, different impact, similarity is iterating over the vaults to trigger...

Vulnerable code:
for (uint256 i = 0; i < accountVaults.length; i++) {
    accountTotalRewards += rewardsBalanceOf(accountVaults[i]);
}

function createVault() external returns (StakeVault clone) {
    clone = StakeVault(Clones.clone(vaultImplementation));
    clone.initialize(msg.sender, stakeManager);
    clone.register();
    emit VaultCreated(address(clone), msg.sender);
}
HIGH severity: Malicious actors can force vaults to exit if they wish to get rewards. **Description:** Rewards are redeemed by iterating over all vaults of an account and calculating the accrued rewards:

There is no way to receive rewards by specifying a singular vault unless you exit the system completely.

The vaults are pushed upon registration in...

Vulnerable code:
function redeemRewards(address account) external onlyNotEmergencyMode whenNotPaused returns (uint256) {
    // ...
    for (uint256 i = 0; i < accountVaults.length; i++) {
        // ...
    }
    // ...
}

function registerVault() external onlyNotEmergencyMode whenNotPaused onlyTrustedCodehash {
    address vault = msg.sender;
    address owner = IStakeVault(vault).owner();

    if (vaultOwners[vault] != address(0)) {
        revert StakeManager__VaultAlreadyRegistered();
    }

    vaultOwners[vault] = owner;
    vaults[owner].push(vault);
    emit VaultRegistered(vault, owner);
}
HIGH severity: Malicious actors can get free rewards if contract gets paused. **Description:** The `StakeVault` contract has a leave functionality with a try/catch block:

It aims to handle bad staking managers that revert, causing a DoS. The issue is that if a legitimate revert can happen (or a forced one, by the caller), then the state in the staking manager will be...

Vulnerable code:
function leave(address _destination) external onlyOwner validDestination(_destination) {
    hasLeft = true;
    try stakeManager.leave() {
        if (lockUntil <= block.timestamp) {
            depositedBalance = 0;
            bool success = STAKING_TOKEN.transfer(_destination, STAKING_TOKEN.balanceOf(address(this)));
            if (!success) {
                revert StakeVault__FailedToLeave();
            }
        }
    } catch {
        if (lockUntil <= block.timestamp) {
            depositedBalance = 0;
            bool success = STAKING_TOKEN.transfer(_destination, STAKING_TOKEN.balanceOf(address(this)));
            if (!success) {
                revert StakeVault__FailedToLeave();
            }
        }
    }
}
HIGH severity: Anyone can dodge reveal delays by providing unrelated account. **Description:** `RLN` provides a commit-reveal functionality to slashing. Firstly, a commit happens:

The hash is based on the private key of the private key and reward recipient address. Thus, a slasher might provide `(bob, hash(PK, slasherAddress))` as the inputs. Then, as he is the first to...

Vulnerable code:
function slashCommit(address account, bytes32 hash) external onlyRole(SLASHER_ROLE) {
    uint256 lastReveal = lastRevealStartTime[account];
    uint256 revealStartTime;

    if (lastReveal == 0 || lastReveal + slashRevealWindowTime < block.timestamp) {
        revealStartTime = block.timestamp;
    } else {
        revealStartTime = lastReveal + slashRevealWindowTime;
    }

    slashCommitments[account][hash] = revealStartTime;
    lastRevealStartTime[account] = revealStartTime;
}

function slashReveal(
    address account,
    bytes32 privateKey,
    address rewardRecipient
)
    external
    onlyRole(SLASHER_ROLE)
{
    /// forge-lint: disable-next-line(asm-keccak256)
    bytes32 hash = keccak256(abi.encodePacked(privateKey, rewardRecipient));
    uint256 revealStartTime = slashCommitments[account][hash];

    if (revealStartTime == 0) {
        revert RLN__InvalidCommitment();
    }

    if (block.timestamp < revealStartTime) {
        revert RLN__RevealWindowNotStarted();
    }

    delete slashCommitments[account][hash];
    slash(privateKey, rewardRecipient);
}
HIGH severity: Any slasher can increase another one's reveal delays. **Description:** Commits in `RLN` are made as follows:

The hash is based on a PK and a reward recipient address. Thus, it seems like an assumption is made that this hash would only ever be used from the legitimate address as he is the one who benefits from having his own reward recipient address......

Vulnerable code:
function slashCommit(address account, bytes32 hash) external onlyRole(SLASHER_ROLE) {
    uint256 lastReveal = lastRevealStartTime[account];
    uint256 revealStartTime;

    if (lastReveal == 0 || lastReveal + slashRevealWindowTime < block.timestamp) {
        revealStartTime = block.timestamp;
    } else {
        revealStartTime = lastReveal + slashRevealWindowTime;
    }

    slashCommitments[account][hash] = revealStartTime;
    lastRevealStartTime[account] = revealStartTime;
}
HIGH severity: Global MP cap invariant can be broken on unstake causing arithmetic underflow and permanent DoS. **Description:** The protocol assumes the invariant `totalMPAccrued <= totalMaxMP` always holds. This invariant is relied upon in the global MP accrual logic `_totalMP()`, which caps newly accrued MP using an unchecked subtraction:

However, this invariant can be violated during normal protocol...

Vulnerable code:
_deltaMpTotal = vault.mpAccrued * amount / vault.stakedBalance;
_deltaMpMax   = vault.maxMP     * amount / vault.stakedBalance;

totalMPAccrued -= _deltaMpTotal;
totalMaxMP     -= _deltaMpMax;

if (totalMPAccrued + accruedMP > totalMaxMP) {
    accruedMP = totalMaxMP - totalMPAccrued;
}
HIGH severity: `ERVotesUpgradeable::getPastTotalSupply` overestimates the value. **Description:** Karma token is used for voting. `ERC20VotesUpgradeable` keeps track of checkpoints: 1) balance of user at certain block, 2) `totalSupply` at certain block. Already minted but not yet distributed tokens are excluded from accounting in both `balanceOf` and `totalSupply` in...

Vulnerable code:
function balanceOf(address account) public view override returns (uint256) {
        if (rewardDistributors.contains(account)) {
            return 0;
        }
        return _balanceOf(account);
    }

    function totalSupply() public view override returns (uint256) {
        uint256 externalSupply = 0;
        uint256 totalDistributorBalance = 0;

        for (uint256 i = 0; i < rewardDistributors.length(); i++) {
            IRewardDistributor distributor = IRewardDistributor(rewardDistributors.at(i));
            externalSupply += distributor.totalRewardsSupply();
            totalDistributorBalance += super.balanceOf(address(distributor));
        }

        if (externalSupply > totalDistributorBalance) {
            externalSupply = totalDistributorBalance;
        }

        // subtract the distributor balances to avoid double counting
        return super.totalSupply() - totalDistributorBalance + externalSupply;
    }

function _mint(address account, uint256 amount) internal virtual override {
        super._mint(account, amount);
        require(totalSupply() <= _maxSupply(), "ERC20Votes: total supply risks overflowing votes");

        _writeCheckpoint(_totalSupplyCheckpoints, _add, amount);
    }

    /**
     * @dev Snapshots the totalSupply after it has been decreased.
     */
    function _burn(address account, uint256 amount) internal virtual override {
        super._burn(account, amount);

        _writeCheckpoint(_totalSupplyCheckpoints, _subtract, amount);
    }
MEDIUM severity: Users can be overslashed in `Karma.sol`. **Description:** `Karma::_calculateSlashAmount` increases slashed amount to `MIN_SLASH_AMOUNT = 1e18` or even full balance:

Problem is that such rounding is applied multiple times in the same action

Suppose following scenario:
1) Current balance is `0.9e18`
2) Virtual balances in...

Vulnerable code:
function _calculateSlashAmount(uint256 balance) internal view returns (uint256) {
        uint256 amountToSlash = Math.mulDiv(balance, slashPercentage, MAX_SLASH_PERCENTAGE);
        if (amountToSlash < MIN_SLASH_AMOUNT) {
            if (balance < MIN_SLASH_AMOUNT) {
                // Not enough balance for minimum slash, slash entire balance
                amountToSlash = balance;
            } else {
                amountToSlash = MIN_SLASH_AMOUNT;
            }
        }
        return amountToSlash;
    }

function _slash(address account, address rewardRecipient) internal virtual returns (uint256) {
        ...

        // first, calculate the total amount to slash from the actual reward tokens
@>      uint256 totalAmountToSlash = _calculateSlashAmount(super.balanceOf(account));

        for (uint256 i = 0; i < rewardDistributors.length(); i++) {
            address distributor = rewardDistributors.at(i);
            uint256 currentDistributorAccountBalance = IRewardDistributor(distributor).rewardsBalanceOfAccount(account);

            // then, calculate the amount to slash from each reward distributor
@>          totalAmountToSlash += _calculateSlashAmount(currentDistributorAccountBalance);

            // turn virtual Karma into real Karma for slashing
            IRewardDistributor(distributor).redeemRewards(account);
        }

        ...

        // Burn the entire slashed amount from the account
        _burn(account, totalAmountToSlash);

        ...
    }