Farseen0/scar-pairs huggingface dataset:
HIGH severity: Native asset cannot be removed using `SmartVaultV4::removeAsset`. **Description:** `SmartVault::removeAsset` allows Smart Vault owners to remove assets from their Vault, including collateral assets so long as the Vault remains fully collateralized. This currently works for ERC-20 collateral tokens; however, there is no handling for the case where `_tokenAddr ==...

Vulnerable code:
function removeAsset(address _tokenAddr, uint256 _amount, address _to) external onlyOwner {
    ITokenManager.Token memory token = getTokenManager().getTokenIfExists(_tokenAddr);
    if (token.addr == _tokenAddr && !canRemoveCollateral(token, _amount)) revert Undercollateralised();
    IERC20(_tokenAddr).safeTransfer(_to, _amount);
    emit AssetRemoved(_tokenAddr, _amount, _to);
}

function removeAsset(address _tokenAddr, uint256 _amount, address _to) external onlyOwner {
    ITokenManager.Token memory token = getTokenManager().getTokenIfExists(_tokenAddr);
    if (token.addr == _tokenAddr && !canRemoveCollateral(token, _amount)) revert Undercollateralised();
+   if(_tokenAddr == address(0)) {
+       (bool sent,) = payable(_to).call{value: _amount}("");
+       if (!sent) revert TransferError();
+   } else {
-   IERC20(_tokenAddr).safeTransfer(_to, _amount);
+        IERC20(_tokenAddr).safeTransfer(_to, _amount);
+   }
    emit AssetRemoved(_tokenAddr, _amount, _to);
}
HIGH severity: Unnecessary call to `SmartVaultV4::usdCollateral` when depositing/withdrawing collateral to/from yield positions. **Description:** When depositing/withdrawing collateral to/from yield positions in `SmartVaultV4`, the Smart Vault is validated to remain sufficiently collateralized and the collateral value is validated to have not dropped by more than 10%:

This logic calls...

Vulnerable code:
if (undercollateralised() || significantCollateralDrop(_preDepositCollateral, usdCollateral())) revert Undercollateralised();

function undercollateralised() public view returns (bool) {
    return minted > maxMintable(usdCollateral());
}
HIGH severity: Cached `_token0` not used. **Description:** In [`SmartVaultYieldManager::_swapToSingleAsset`](https://github.com/the-standard/smart-vault/blob/c6837d4a296fe8a6e4bb5e0280a66d6eb8a40361/contracts/SmartVaultYieldManager.sol#L114-L117), the cached address `_token0` is not used in the condition of the ternary...

Vulnerable code:
address _token0 = IHypervisor(_hypervisor).token0();
address _unwantedToken = IHypervisor(_hypervisor).token0() == _wantedToken ?
    IHypervisor(_hypervisor).token1() :
    _token0;
HIGH severity: The `fetchRewards` function in `CurveDepositToken` and `ConvexDepositToken` causes a loss of accrued rewards. **Description:** The `fetchRewards` function in `CurveDepositToken` and `ConvexDepositToken` [fails](https://github.com/Bima-Labs/bima-v1-core/blob/09461f0d22556e810295b12a6d7bc5c0efec4627/contracts/staking/Curve/CurveDepositToken.sol#L218-L221) to call `_updateIntegrals` prior to calling...

Vulnerable code:
function fetchRewards() external {
    require(block.timestamp / 1 weeks >= periodFinish / 1 weeks, "Can only fetch once per week");
+   _updateIntegrals(address(0), 0, totalSupply);
    _fetchRewards();
}

+ if (account != address(0)) {
    uint256 integralFor = rewardIntegralFor[account][i];
    if (integral > integralFor) {
        storedPendingReward[account][i] += uint128((balance * (integral - integralFor)) / 1e18);
        rewardIntegralFor[account][i] = integral;
    }
+ }
HIGH severity: Impossible to liquidate borrower when a `TroveManager` instance only has 1 active borrower. **Description:** When a `TroveManager` instance only has 1 active borrower it is impossible to liquidate that borrower since `LiquidationManager::liquidateTroves` only executes code within the [while](https://github.com/Bima-Labs/bima-v1-core/blob/09461f0d22556e810295b12a6d7bc5c0efec4627/contracts/c...

Vulnerable code:
uint256 troveCount = troveManager.getTroveOwnersCount();
...
while (trovesRemaining > 0 && troveCount > 1) {
...
if (trovesRemaining > 0 && !troveManagerValues.sunsetting && troveCount > 1) {

require(totals.totalDebtInSequence > 0, "TroveManager: nothing to liquidate");

uint256 troveCount = troveManager.getTroveOwnersCount();
...
while (troveIter < length && troveCount > 1) {
...
if (troveIter < length && troveCount > 1) {
HIGH severity: Loss of user locked voting tokens due to unsafe downcast overflow. **Description:** `TokenLocker::AccountData` [stores](https://github.com/Bima-Labs/bima-v1-core/blob/09461f0d22556e810295b12a6d7bc5c0efec4627/contracts/dao/TokenLocker.sol#L41-L49) the account's current `locked`, `unlocked` and `frozen` balances using `uint32`:

Inside `TokenLocker::_lock` the...

Vulnerable code:
struct AccountData {
    // Currently locked balance. Each week the lock weight decays by this amount.
    uint32 locked;
    // Currently unlocked balance (from expired locks, can be withdrawn)
    uint32 unlocked;
    // Currently "frozen" balance. A frozen balance is equivalent to a `MAX_LOCK_WEEKS` lock,
    // where the lock weight does not decay weekly. An account may have a locked balance or a
    // frozen balance, never both at the same time.
    uint32 frozen;

accountData.locked = uint32(accountData.locked + _amount);

uint256 locked = accountData.locked;
MEDIUM severity: When `BabelVault` uses an `EmissionSchedule` but receivers have no voting weight, the vault's unallocated supply will decrease even though no tokens are being allocated. **Description:** When `BabelVault` uses an `EmissionSchedule` but receivers have no voting weight, the vault's unallocated supply will decrease even though no tokens are being allocated.

**Impact:** Tokens are effectively lost since the vault's unallocated supply decreases but no tokens are...

Vulnerable code:
function test_allocateNewEmissions_unallocatedTokensDecreasedButZeroAllocated() external {
    // first need to fund vault with tokens
    test_setInitialParameters();

    // owner registers receiver
    address receiver = address(mockEmissionReceiver);
    vm.prank(users.owner);
    assertTrue(babelVault.registerReceiver(receiver, 1));

    // warp time by 1 week
    vm.warp(block.timestamp + 1 weeks);

    // cache state prior to allocateNewEmissions
    uint256 RECEIVER_ID = 1;
    uint16 systemWeek = SafeCast.toUint16(babelVault.getWeek());

    // entire supply still not allocated
    uint256 initialUnallocated = babelVault.unallocatedTotal();
    assertEq(initialUnallocated, INIT_BAB_TKN_TOTAL_SUPPLY);

    // receiver calls allocateNewEmissions
    vm.prank(receiver);
    uint256 allocated = babelVault.allocateNewEmissions(RECEIVER_ID);

    // verify BabelVault::totalUpdateWeek current system week
    assertEq(babelVault.totalUpdateWeek(), systemWeek);

    // verify unallocated supply reduced by weekly emission percent
    uint256 firstWeekEmissions = INIT_BAB_TKN_TOTAL_SUPPLY*INIT_ES_WEEKLY_PCT/MAX_PCT;
    assertTrue(firstWeekEmissions > 0);
    assertEq(babelVault.unallocatedTotal(), initialUnallocated - firstWeekEmissions);

    // verify emissions correctly set for current week
    assertEq(babelVault.weeklyEmissions(systemWeek), firstWeekEmissions);

    // verify BabelVault::lockWeeks reduced correctly
    assertEq(babelVault.lockWeeks(), INIT_ES_LOCK_WEEKS-INIT_ES_LOCK_DECAY_WEEKS);

    // verify receiver active and last processed week = system week
    (, bool isActive, uint16 updatedWeek) = babelVault.idToReceiver(RECEIVER_ID);
    assertEq(isActive, true);
    assertEq(updatedWeek, systemWeek);

    // however even though BabelVault::unallocatedTotal was reduced by the
    // first week emissions, nothing was allocated to the receiver
    assertEq(allocated, 0);

    // this is because EmissionSchedule::getReceiverWeeklyEmissions calls
    // IncentiveVoting::getReceiverVotePct which looks back 1 week, and receiver
    // had no voting weight and there was no total voting weight at all in that week
    assertEq(incentiveVoting.getTotalWeightAt(systemWeek-1), 0);
    assertEq(incentiveVoting.getReceiverWeightAt(RECEIVER_ID, systemWeek-1), 0);

    // tokens were effectively lost since the vault's unallocated supply decreased
    // but no tokens were actually allocated to receivers since there was no
    // voting weight
}
MEDIUM severity: Disabled receiver can lose tokens that were allocated for past weeks if not regularly claiming emissions. **Description:** When a receiver is disabled by `Vault::setReceiverIsActive`, they can lose tokens which they were eligible to receive in past weeks if they have not been regularly claiming emissions.

**Impact:** A disabled receiver can lose tokens which they were eligible for.

**Proof of...

Vulnerable code:
function setReceiverIsActive(uint256 id, bool isActive) external onlyOwner returns (bool success) {
    // revert if receiver id not associated with an address
    require(idToReceiver[id].account != address(0), "ID not set");

    // update storage - isActive status, address remains the same
    idToReceiver[id].isActive = isActive;

    emit ReceiverIsActiveStatusModified(id, isActive);

    success = true;
}

if (receiver.isActive) {
    allocated[msg.sender] += amount;

    emit IncreasedAllocation(msg.sender, amount);
}
// otherwise return allocation to the unallocated supply
else {
    uint256 unallocated = unallocatedTotal + amount;
    unallocatedTotal = SafeCast.toUint128(unallocated);
    ...
}
MEDIUM severity: `StorkOracleWrapper` downscales 18 decimal price to 8 decimals then `PriceFeed` upscales to 18 decimals resulting in inaccurate price. **Description:** `StorkOracleWrapper` returns hard-coded 8 decimals:

And its functions `getRoundData` and `latestRoundData` always downscale the native 18-decimal price to 8 decimals:

But `PriceFeed` always converts Oracle values to 18 decimals:

**Impact:** Asset prices used by the protocol...

Vulnerable code:
function decimals() external pure returns (uint8 dec) {
    dec = 8;
}

uint256 public constant TARGET_DIGITS = 18;

function _scalePriceByDigits(uint256 _price, uint256 _answerDigits) internal pure returns (uint256 scaledPrice) {
    if (_answerDigits == TARGET_DIGITS) {
        scaledPrice = _price;
    } else if (_answerDigits < TARGET_DIGITS) {
        // Scale the returned price value up to target precision
        scaledPrice = _price * (10 ** (TARGET_DIGITS - _answerDigits));
    } else {
        // Scale the returned price value down to target precision
        scaledPrice = _price / (10 ** (_answerDigits - TARGET_DIGITS));
    }
}
HIGH severity: Implementation contracts should inherit from their interfaces enabling compile-time checks ensuring implementations correctly implement their interfaces. **Description:** Implementation contracts don't inherit from their interfaces which prevents compile-time checks that interfaces are correctly implemented. The interfaces and implementations also contain a lot of copy & paste duplicate definitions.

For example, examine the interface...

Vulnerable code:
function setOracle(
    address _token,
    address _chainlinkOracle,
    bytes4 sharePriceSignature,
    uint8 sharePriceDecimals,
    bool _isEthIndexed
) external;

function RESPONSE_TIMEOUT() external view returns (uint256);

function oracleRecords(
    address
)
    external
    view
    returns (
        address chainLinkOracle,
        uint8 decimals,
        bytes4 sharePriceSignature,
        uint8 sharePriceDecimals,
        bool isFeedWorking,
        bool isEthIndexed
    );

struct OracleRecord {
    IAggregatorV3Interface chainLinkOracle;
    uint8 decimals;
    // @audit extra `heartbeat` members breaks interface `IPriceFeed::oracleRecords`
    uint32 heartbeat;
    bytes4 sharePriceSignature;
    uint8 sharePriceDecimals;
    bool isFeedWorking;
    bool isEthIndexed;

// @audit different name breaks interface `IPriceFeed::RESPONSE_TIMEOUT`
uint256 public constant RESPONSE_TIMEOUT_BUFFER = 1 hours;

function setOracle(
    address _token,
    address _chainlinkOracle,
    // @audit extra `heartbeat` members breaks interface `IPriceFeed::setOracle`
    uint32 _heartbeat,
    bytes4 sharePriceSignature,
    uint8 sharePriceDecimals,
    bool _isEthIndexed
) public onlyOwner {
}
LOW severity: `DebtToken::flashLoan` fees can be bypassed by borrowing in small amounts. **Description:** `DebtToken::flashLoan` fees can be bypassed by borrowing in small amounts to trigger [rounding down to zero](https://dacian.me/precision-loss-errors#heading-rounding-down-to-zero) precision loss in fee calculation. Since this function allows re-entrancy, the function can be...

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

// test setup
import {TestSetup, DebtToken} from "../TestSetup.sol";

contract DebtTokenTest is TestSetup {

    function test_flashLoan() external {
        // entire supply initially available to borrow
        assertEq(debtToken.maxFlashLoan(address(debtToken)), type(uint256).max);

        // expected fee for borrowing 1e18
        uint256 borrowAmount = 1e18;
        uint256 expectedFee  = borrowAmount * debtToken.FLASH_LOAN_FEE() / 10000;

        // fee should be > 0
        assertTrue(expectedFee > 0);

        // fee should exactly equal
        assertEq(debtToken.flashFee(address(debtToken), borrowAmount), expectedFee);

        // exploit rounding down to zero precision loss to get free flash loans
        // by borrowing in small amounts
        borrowAmount = 1111;
        assertEq(debtToken.flashFee(address(debtToken), borrowAmount), 0);

        // as DebtToken::flashLoan allows re-entrancy, the function can be re-entered
        // multiple times to borrow larger amounts at zero fee
    }
}

function _flashFee(uint256 amount) internal pure returns (uint256 fee) {
    fee = (amount * FLASH_LOAN_FEE) / 10000;
    require(fee > 0, "ERC20FlashMint: amount too small");
}
LOW severity: Easily bypass `setGuardian` proposal passing requirement. **Description:** `AdminVoting::createNewProposal` calls `_isSetGuardianPayload` to detect whether a proposal contains a call to `IBabelCore.setGuardian`. If `true` then it requires a 50.1% majority in order for the `setGuardian` proposal to succeed, otherwise it uses the configured proposal pass...

Vulnerable code:
function test_createNewProposal_setGuardian() external {
    // create a setGuardian proposal that also contains a dummy proposal
    AdminVoting.Action[] memory payload = new AdminVoting.Action[](2);
    payload[0].target = address(0x0);
    payload[0].data   = abi.encode("");
    payload[1].target = address(babelCore);
    payload[1].data   = abi.encodeWithSelector(IBabelCore.setGuardian.selector, users.user1);

    // lock up user tokens to receive voting power
    vm.prank(users.user1);
    tokenLocker.lock(users.user1, INIT_BAB_TKN_TOTAL_SUPPLY, 52);

    // warp forward BOOTSTRAP_PERIOD so voting power becomes active
    // and setGuardian proposals are allowed
    vm.warp(block.timestamp + adminVoting.BOOTSTRAP_PERIOD());

    // create the proposal
    vm.prank(users.user1);
    adminVoting.createNewProposal(users.user1, payload);

    // verify requiredWeight is 50.1% majority for setGuardian proposals
    assertEq(adminVoting.getProposalRequiredWeight(0),
             (tokenLocker.getTotalWeight() * adminVoting.SET_GUARDIAN_PASSING_PCT()) /
             adminVoting.MAX_PCT());

    // fails since the first dummy payload evaded detection of the
    // second setGuardian payload
}

function _containsSetGuardianPayload(uint256 payloadLength, Action[] memory payload) internal view returns (bool) {
    for(uint256 i; i<payloadLength; i++) {
        bytes memory data = payload[i].data;

        // Extract the call sig from payload data
        bytes4 sig;
        assembly {
            sig := mload(add(data, 0x20))
        }

        if(sig == IBabelCore.setGuardian.selector) return true;
    }

    return false;
}
LOW severity: Don't allow cancellation of executed or cancelled proposals. **Description:** `AdminVoting::cancelProposal` and `InterimAdmin::cancelProposal` allow cancellation of executed or cancelled proposals which doesn't make logical sense and may mislead guardian as to the exact state of the proposal.

**Recommended Mitigation:** Prevent cancellation of executed or...

Vulnerable code:
function cancelProposal(uint256 id) external {
    require(msg.sender == babelCore.guardian(), "Only guardian can cancel proposals");
    require(id < proposalData.length, "Invalid ID");

    Action[] storage payload = proposalPayloads[id];
    require(!_containsSetGuardianPayload(payload.length, payload), "Guardian replacement not cancellable");
+   require(!proposalData[id].processed, "Already processed");
    proposalData[id].processed = true;
    emit ProposalCancelled(id);
}
LOW severity: `TokenLocker::getTotalWeightAt` should loop until input `week` not `systemWeek`. **Description:** `TokenLocker::getTotalWeightAt` should loop until input `week` not `systemWeek`:

The current implementation returns the wrong output when `week < systemWeek` and the previous weekly writes have not occurred which triggers processing of the final `while` loop, since it counts all...

Vulnerable code:
- while (updatedWeek < systemWeek) {
+ while (updatedWeek < week) {
LOW severity: `StabilityPool` user functions panic revert if more than 256 collaterals are enabled. **Description:** `StabilityPool` has a number of mappings where the value component is an array with length 256:

There are a number of internal functions which iterate over every collateral then update these arrays, eg:

Additionally `StabilityPool::enableCollateral` has no limit on the maximum...

Vulnerable code:
mapping(address depositor => uint256[256] deposits) public depositSums;
mapping(address depositor => uint80[256] gains) public collateralGainsByDepositor;
mapping(uint128 epoch => mapping(uint128 scale => uint256[256] sumS)) public epochToScaleToSums;
mapping(uint128 epoch => mapping(uint128 scale => uint256 sumG)) public epochToScaleToG;

function _updateSnapshots(address _depositor, uint256 _newValue) internal {
    uint256 length;
    if (_newValue == 0) {
        delete depositSnapshots[_depositor];

        length = collateralTokens.length;
        for (uint256 i; i < length; i++) {
            // @audit will panic revert if >= 257 collaterals are enabled
            depositSums[_depositor][i] = 0;
        }

function test_provideToSP_panicWhen257Collaterals() external {
    for(uint160 i=1; i<=256; i++) {
        address newCollateral = address(i);

        vm.prank(address(factory));
        stabilityPool.enableCollateral(IERC20(newCollateral));
    }

    assertEq(stabilityPool.getNumCollateralTokens(), 257);

    // mint user1 some tokens
    uint256 tokenAmount = 100e18;
    vm.prank(address(borrowerOps));
    debtToken.mint(users.user1, tokenAmount);
    assertEq(debtToken.balanceOf(users.user1), tokenAmount);

    // user1 deposits them into stability pool
    vm.prank(users.user1);
    stabilityPool.provideToSP(tokenAmount);
    // fails with panic: array out-of-bounds access
}
LOW severity: `setGuardian` proposals may incorrectly set lower passing percent if current default is greater than hard-coded value for guardian proposals. **Description:** `AdminVoting::createNewProposal` has a check that for `setGuardian` proposals it always uses the hard-coded `SET_GUARDIAN_PASSING_PCT` as the passing percent:

The idea is that `setGuardian` proposals are very sensitive proposals and hence should require a 50.1% majority to pass,...

Vulnerable code:
if (_containsSetGuardianPayload(payload.length, payload)) {
    // prevent changing guardians during bootstrap period
    require(block.timestamp > startTime + BOOTSTRAP_PERIOD, "Cannot change guardian during bootstrap");

    // enforce 50.1% majority for setGuardian proposals
    proposalPassPct = SET_GUARDIAN_PASSING_PCT;
}
// otherwise for ordinary proposals enforce standard configured passing %
else proposalPassPct = passingPct;

- proposalPassPct = SET_GUARDIAN_PASSING_PCT;
+ proposalPassPct = BabelMath._max(SET_GUARDIAN_PASSING_PCT, passingPct);
LOW severity: Prevent panic from division by zero in `BabelBase::_requireUserAcceptsFee`. **Description:** Prevent panic from division by zero in `BabelBase::_requireUserAcceptsFee`:

**Bima:**
Fixed in commit [fe67023](https://github.com/Bima-Labs/bima-v1-core/commit/fe670238f6290cec1d803dc68160d96592fc5346).

**Cyfrin:** Verified.

Vulnerable code:
function _requireUserAcceptsFee(uint256 _fee, uint256 _amount, uint256 _maxFeePercentage) internal pure {
+   require(_amount > 0, "Amount must be greater than zero");
    uint256 feePercentage = (_fee * BIMA_DECIMAL_PRECISION) / _amount;
    require(feePercentage <= _maxFeePercentage, "Fee exceeded provided maximum");
}
LOW severity: `StabilityPool` reward calculation loses small amounts of vault emission rewards. **Description:** `StabilityPool` can earn token emission rewards from `BabelVault` if users vote for it to do so, and these rewards are distributed to users who deposit into `StabilityPool`. The mechanism for calculating depositors' reward share appears to result in small amounts of lost token...

Vulnerable code:
function test_claimReward_smallAmountOfStabilityPoolRewardsLost() external {
    // setup vault giving user1 half supply to lock for voting power
    uint256 initialUnallocated = _vaultSetupAndLockTokens(INIT_BAB_TKN_TOTAL_SUPPLY/2);

    // user votes for stability pool to get emissions
    IIncentiveVoting.Vote[] memory votes = new IIncentiveVoting.Vote[](1);
    votes[0].id = stabilityPool.SP_EMISSION_ID();
    votes[0].points = incentiveVoting.MAX_POINTS();

    vm.prank(users.user1);
    incentiveVoting.registerAccountWeightAndVote(users.user1, 52, votes);

    // user1 and user 2 both deposit 10K into the stability pool
    uint96 spDepositAmount = 10_000e18;
    _provideToSP(users.user1, spDepositAmount, 1);
    _provideToSP(users.user2, spDepositAmount, 1);

    // warp time by 1 week
    vm.warp(block.timestamp + 1 weeks);

    // calculate expected first week emissions
    uint256 firstWeekEmissions = initialUnallocated*INIT_ES_WEEKLY_PCT/BIMA_100_PCT;
    assertTrue(firstWeekEmissions > 0);
    assertEq(babelVault.unallocatedTotal(), initialUnallocated);

    uint16 systemWeek = SafeCast.toUint16(babelVault.getWeek());

    // no rewards in the same week as emissions
    vm.prank(users.user1);
    uint256 userReward = stabilityPool.claimReward(users.user1);
    assertEq(userReward, 0);

    // verify emissions correctly set in BabelVault for first week
    assertEq(babelVault.weeklyEmissions(systemWeek), firstWeekEmissions);

    // warp time by 1 week
    vm.warp(block.timestamp + 1 weeks);

    // rewards for the first week can be claimed now
    vm.prank(users.user1);
    userReward = stabilityPool.claimReward(users.user1);

    // verify user1 receives half of the emissions
    assertEq(firstWeekEmissions/2, 268435455937500000000000000);
    assertEq(userReward,           268435455937499999999890000);

    // user 2 claims their reward
    vm.prank(users.user2);
    userReward = stabilityPool.claimReward(users.user2);
    assertEq(userReward,           268435455937499999999890000);

    // firstWeekEmissions = 536870911875000000000000000
    // userReward * 2     = 536870911874999999999780000
    //
    // a small amount of rewards was not distributed and is effectively lost
}
HIGH severity: Use named mapping parameters. **Description:** Solidity 0.8.18 [introduced](https://soliditylang.org/blog/2023/02/01/solidity-0.8.18-release-announcement/) named `mapping` parameters; use this feature for clearer mappings:

**Bima:**
Fixed in commit...

Vulnerable code:
PriceFeed.sol
StabilityPool.sol

dao/AdminVoting.sol
dao/AllocationVesting.sol
dao/BoostCalculator.sol
dao/IncentiveVoting.sol
dao/TokenLocker.sol
dao/Vault.sol