Farseen0/scar-pairs huggingface dataset:
HIGH severity: Migrating collateral will leave dust amounts of source tokens behind. **Description:** When migrating collateral, the user provides two separate amounts related to how much of the source market tokens should be migrated: `migrationAmount` and `swapParams.fromAssetAmount`.

- `migrationAmount` is the number of `qiTokens` (Compound V2 `CTokens`) to redeem. This is...

Vulnerable code:
// Redeem the migration amount from the source market
if (migrationAmount == type(uint256).max) {
    // Get the QiToken balance of the source market
    migrationAmount = IMinimalQiToken(sourceMarket).balanceOf(user);
}
// Transfer the QiTokens to the contract
IERC20(sourceMarket).safeTransferFrom(user, address(this), migrationAmount);
// Redeem the QiTokens from the source market
if (IMinimalQiToken(sourceMarket).redeem(migrationAmount) != 0) revert RedeemFailed();

// Swap the underlying asset from the source market to the target market
_swap(
    _swapParams.fromAsset,
    _swapParams.toAsset,
    _swapParams.fromAssetAmount,
    _swapParams.minToAssetAmount,
    _swapParams.data
);

// Swap the underlying asset from the source market to the target market
  _swap(
      _swapParams.fromAsset,
      _swapParams.toAsset,
-     _swapParams.fromAssetAmount,
+     IERC20(_swapParams.fromAsset).balanceOf(address(this))
      _swapParams.minToAssetAmount,
      _swapParams.data
  );
HIGH severity: Unbounded loop in `CollateralMigrator::_removeMigrationConfigData` may hinder contract maintainability. **Description:** [`CollateralMigrator::_removeMigrationConfigData`](https://github.com/woof-software/benqi-collateral-migrator/blob/d91ce3dbf56d3939d54520f6f41afeebbbfc069a/contracts/CollateralMigrator.sol#L790-L827)  contains an unbounded loop when removing migration configurations:

This...

Vulnerable code:
function _removeMigrationConfigData(address market) private {
    uint32 length = uint32(_markets.length);
    bool found = false;

    for (uint256 i = 0; i < length; i++) {
        if (_markets[i] == market) {
            found = true;
            // remove the market from the list
            _markets[i] = _markets[length - 1];
            _markets.pop();

            // remove the market data and flash data
            delete _migrationConfigData[market];

            emit MigrationConfigDataRemoved(market);
            break;
        }
    }
    // revert if the market is not found
    if (!found) revert MarketIsNotSupported(market);
}

function _removeMigrationConfigData(uint256 index) external onlyRole(DEFAULT_ADMIN_ROLE) {
    address market = _markets[index];

    _markets[index] = _markets[_markets.length - 1];
    _markets.pop();

    delete _migrationConfigData[market];

    emit MigrationConfigDataRemoved(market);
}
HIGH severity: Unused imports. **Description:** There are two unused imports:
* [`CommonErrors`, `CollateralMigrator.sol#17`](https://github.com/woof-software/benqi-collateral-migrator/blob/d91ce3dbf56d3939d54520f6f41afeebbbfc069a/contracts/CollateralMigrator.sol#L17):
  
* [`ISwapper`,...

Vulnerable code:
import {CommonErrors} from "./errors/CommonErrors.sol";

import {ISwapper} from "../interfaces/1Inch-v6/ISwapper.sol";
HIGH severity: NatSpec `@custom:reverts` inconsistencies. **Description:** Below are some inconsistencies in how the NatSpec `@custom:reverts` are formatted:

* [`CollateralMigrator::LBFlashLoanCallback`](https://github.com/woof-software/benqi-collateral-migrator/blob/d91ce3dbf56d3939d54520f6f41afeebbbfc069a/contracts/CollateralMigrator.sol#L438-L442):...

Vulnerable code:
* @custom:reverts IncorrectToAsset Thrown if the `toAsset` in `swapParams` does not match the target market's base token.
  * @custom:reverts RedeemFailed Thrown if the QiToken redemption process fails.
  * - {MintFailed}: Thrown if the minting of tokens to the target market fails.
  * @custom:reverts MintFailed Thrown if the minting of tokens to the target market fails.
  * @custom:reverts InvalidCallbackHash Thrown if the callback data hash does not match the stored hash.

/**
   * @notice Initializes the SwapModule with the address of the 1Inch router.
   * @param _swapRouter The address of the 1Inch router contract.
   * @dev Reverts with {InvalidZeroAddress} if `_swapRouter` is the zero address.
   */
  constructor(address _swapRouter) {
      if (_swapRouter == address(0)) {
          revert InvalidZeroAddress();
      }
      SWAP_ROUTER = _swapRouter;
  }

* - {SwapFailed} if the call to the 1Inch router fails.
  * - {IncorrectSwapAmount} if the resulting balance does not increase.
  * - {OutputLessThanMinAmount} if the received amount is less than the specified minimum.
LOW severity: Missing `receive` function to reject direct ETH transfers in messager contracts. **Description:** The messager contracts (`MTokenMessager`, `MTokenMessagerLZ`, `MTokenMessagerV2`) are designed to receive the bridging fee in native token but none of them implemented a `receive()` function to handle direct ETH transfers. Without this function, users can accidentally send ETH to...

Vulnerable code:
3 contract MTokenMessagerBase {
4
5     address public ccipClient;//@audit-info MToken
6
7     constructor(address _ccipClient){
8         ccipClient = _ccipClient;
9     }
+
+     receive() external payable {
+         revert("ETH transfers not accepted");
+     }
10 }
LOW severity: Cross-chain blocked recipients aren't properly handled. **Description:** The `MToken` contract implements a blocking mechanism to prevent certain addresses from interacting with the token. However, the cross-chain functionality doesn't properly handle blocked addresses.

There are two key issues:

1. In `MToken::msgOfCcSendToken`, the contract checks...

Vulnerable code:
369:    function msgOfCcSendToken(
370:        address sender,
371:        address receiver,
372:        uint256 value
373:    ) public view returns (bytes memory message) {
374:        _checkBlocked(sender);
375:        _checkBlocked(receiver);//@audit-issue receiver is not on the same chain, so this check does not make sense
376:        return abi.encode(TagSendToken, abi.encode(sender, receiver, value));
377:    }

415:    function ccReceiveToken(bytes memory message) internal {
416:        (address sender, address receiver, uint value) = abi.decode(
417:            message,
418:            (address, address, uint)
419:        );
420:        _mint(receiver, value);//@audit-issue should check if receiver is blocked, might need to manage the funds sent to the blocked address
421:        emit CCReceiveToken(sender, receiver, value);
422:    }

// Test cross-chain sending to a blocked address
    function testCrossChainSendToken_ToBlockedAddress() public {
        // Mint some tokens to user1
        uint256 amount = 100 * 10**18;
        mintTokens(user1, amount);

        // Block user2 on the destination chain
        vm.prank(operator);
        remoteChainMToken.addToBlockedList(user2);

        // User1 tries to send tokens cross-chain to blocked user2
        vm.startPrank(user1);
        mtoken.approve(address(mockMessager), amount);

        // When sending to a blocked address, the send may succeed but the tokens should never reach the destination
        mockMessager.sendTokenToChain{value: 0.01 ether}(
            CHAIN_SELECTOR_2,
            address(remoteChainMToken),
            user2,
            amount,
            ""
        );
        vm.stopPrank();

        // Check that user1's tokens are gone (burned in the sending process)
        assertEq(mtoken.balanceOf(user1), 0, "Tokens should be burned on source chain");

        // The blocked user should NOT receive any tokens
        // assertEq(remoteChainMToken.balanceOf(user2), 0, "Blocked user should not receive tokens");
    }
MEDIUM severity: Use named returns especially for `memory` outputs. **Description:** Use named returns especially for `memory` outputs, eg in `MTokenMessager::calculateCCSendTokenFeeAndMessage`:

Also applies to:
* `MTokenMessager::calculateCcSendMintBudgetFeeAndMessage`
* `MTokenMessager::sendDataToChain` where obsolete `return` can be removed
* the same...

Vulnerable code:
function calculateCCSendTokenFeeAndMessage(
        uint64 destinationChainSelector,
        address messageReceiver,
        address sender,
        address recipient,
        uint value,
        bytes calldata extraArgs
    )
        public
        view
        returns (uint256 fee, Client.EVM2AnyMessage memory evm2AnyMessage)
    {
        bytes memory data = ccipClient.msgOfCcSendToken(
            sender,
            recipient,
            value
        );
-       return
+       (fee, evm2AnyMessage) =
            getFeeAndMessage(
                destinationChainSelector,
                messageReceiver,
                extraArgs,
                data
            );
    }
MEDIUM severity: Collector can add `CreatorStory`, corrupting the provenance of an artwork. **Description:** The purpose of the `IStory` interface is to allow 3 different entities (Admin, Creator and Collectors) to add "Stories" about a given artwork (NFT) which [describes the provenance of the...

Vulnerable code:
/// @notice Function to let the creator add a story to any token they have created
/// @dev This function MUST implement logic to restrict access to only the creator
function addCreatorStory(uint256 tokenId, string calldata creatorName, string calldata story) external;

function addCreatorStory(uint256 tokenId, string calldata, /*creatorName*/ string calldata story)
    external
    onlyTokenOwner(tokenId)
{
    emit CreatorStory(tokenId, msg.sender, msg.sender.toHexString(), story);
}
HIGH severity: Allow custom Creator and Collector names to be emitted in `IStory` events to build artwork provenance. **Description:** The `IStory` interface is designed to allow custom names to be emitted for the Creator and Collector events. Here is an [example](https://www.transient.xyz/nfts/base/0x6c81306129b3cc63b0a6c7cec3dd50721ac378fe/9) where a Creator has used the custom name `lytke`.

But in CryptoArt's.....

Vulnerable code:
function addCollectionStory(string calldata, /*creatorName*/ string calldata story) external onlyOwner {
    emit CollectionStory(msg.sender, msg.sender.toHexString(), story);
}

/// @inheritdoc IStory
function addCreatorStory(uint256 tokenId, string calldata, /*creatorName*/ string calldata story)
    external
    onlyTokenOwner(tokenId)
{
    emit CreatorStory(tokenId, msg.sender, msg.sender.toHexString(), story);
}

/// @inheritdoc IStory
function addStory(uint256 tokenId, string calldata, /*collectorName*/ string calldata story)
    external
    onlyTokenOwner(tokenId)
{
    emit Story(tokenId, msg.sender, msg.sender.toHexString(), story);
}
HIGH severity: `IER` specification requires `hasPinnedTokenURI` to revert for non-existent `tokenId`. **Description:** Per the specification of `IERC7160`:

But the implementation of `hasPinnedTokenURI` doesn't revert for tokens which don't exist, instead it will simply return `false` or even return `true` if a token was burned when the value was true since burning doesn't delete...

Vulnerable code:
/// @notice Check on-chain if a token id has a pinned uri or not
/// @dev This call MUST revert if the token does not exist
function hasPinnedTokenURI(uint256 tokenId) external view returns (bool pinned);

function hasPinnedTokenURI(uint256 tokenId) external view returns (bool) {
    return _hasPinnedTokenURI[tokenId];
}

-    function hasPinnedTokenURI(uint256 tokenId) external view returns (bool) {
+    function hasPinnedTokenURI(uint256 tokenId) external view onlyIfTokenExists(tokenId) returns (bool) {
HIGH severity: `IER` specification requires `pinTokenURI` to revert for non-existent `tokenId`. **Description:** Per the specification of `IERC7160`:

But the implementation of `pinTokenURI` doesn't revert for tokens which don't exist, since `_tokenURIs[tokenId].length` will always equal 2 even for non-existent `tokenId`:

**Recommended Mitigation:** Use the `onlyIfTokenExists`...

Vulnerable code:
/// @notice Pin a specific token uri for a particular token
/// @dev This call MUST revert if the token does not exist
function pinTokenURI(uint256 tokenId, uint256 index) external;

// mapping value always has fixed array size of 2
mapping(uint256 tokenId => string[2] tokenURIs) private _tokenURIs;

function pinTokenURI(uint256 tokenId, uint256 index) external onlyOwner {
    if (index >= _tokenURIs[tokenId].length) {
        revert Error.Token_IndexOutOfBounds(tokenId, index, _tokenURIs[tokenId].length - 1);
    }

    _pinnedURIIndex[tokenId] = index;

    emit TokenUriPinned(tokenId, index);
    emit MetadataUpdate(tokenId);
}

-    function pinTokenURI(uint256 tokenId, uint256 index) external onlyOwner {
+    function pinTokenURI(uint256 tokenId, uint256 index) external onlyIfTokenExists(tokenId) onlyOwner {
HIGH severity: In `tokenURI` avoid copying entire `_tokenURIs[tokenId]` from `storage` into `memory`. **Description:** `tokenURI` only uses the "pinned" URI index so there's no reason to copy both token URIs from `storage` to `memory`. Simply use a `storage` reference like this:

**CryptoArt:**
Fixed in commit...

Vulnerable code:
function tokenURI(uint256 tokenId)
        public
        view
        override(ERC721Upgradeable)
        onlyIfTokenExists(tokenId)
        returns (string memory)
    {
-       string[2] memory uris = _tokenURIs[tokenId];
+       string[2] storage uris = _tokenURIs[tokenId];
        string memory uri = uris[_getTokenURIIndex(tokenId)];

        if (bytes(uri).length == 0) {
            revert Error.Token_NoURIFound(tokenId);
        }

        return string.concat(_baseURI(), uri);
    }
HIGH severity: `burn` should delete `tokenURI` related data and emit `TokenUriUnpinned` event. **Description:** The `burn` function should delete `tokenURI` related data and emit `TokenUriUnpinned` event:

This provides a gas refund as part of the burn and also removes token data that should no longer exist. It also prevents `hasPinnedTokenURI` from returning `true` for a burned token since.....

Vulnerable code:
function burn(uint256 tokenId) public override whenNotPaused {
        // require sender is owner or approved has been removed as the internal burn function already checks this
        ERC2981Upgradeable._resetTokenRoyalty(tokenId);
        ERC721BurnableUpgradeable.burn(tokenId);
        emit Burned(tokenId);
+       emit TokenUriUnpinned(tokenId);
+       delete _tokenURIs[tokenId];
+       delete _pinnedURIIndex[tokenId];
+       delete _hasPinnedTokenURI[tokenId];
    }
HIGH severity: InfraVault's Permissionless Reward Claiming Can Allow Anyone to Lock Rewards in the MetaVault. **Description:** The `InfraredBGTMetaVault` relies on `_performDepositRewardByRewardType` to handle newly claimed rewards by either depositing them into DolomiteMargin or sending them directly to the vault owner. However, the onchain Infrared vault...

Vulnerable code:
/// @inheritdoc IMultiRewards
function getRewardForUser(address _user)
    public
    nonReentrant
    updateReward(_user)
{
    onReward();
    uint256 len = rewardTokens.length;
    for (uint256 i; i < len; i++) {
        address _rewardsToken = rewardTokens[i];
        uint256 reward = rewards[_user][_rewardsToken];
        if (reward > 0) {
            (bool success, bytes memory data) = _rewardsToken.call{
                gas: 200000
            }(
                abi.encodeWithSelector(
                    ERC20.transfer.selector, _user, reward
                )
            );
            if (success && (data.length == 0 || abi.decode(data, (bool)))) {
                rewards[_user][_rewardsToken] = 0;
                emit RewardPaid(_user, _rewardsToken, reward);
            } else {
                continue;
            }
        }
    }
}

function _performDepositRewardByRewardType(
        IMetaVaultRewardTokenFactory _factory,
        IBerachainRewardsRegistry.RewardVaultType _type,
        address _token,
        uint256 _amount
    ) internal {
++ _amount += IERC20(token).balanceOf(address(this));
}
MEDIUM severity: User rewards may remain unclaimed after calling `InfraredBGTIsolationModeTokenVaultV1::exit()` function. **Description:** When a user calls `InfraredBGTIsolationModeTokenVaultV1::exit` in a scenario where rewards are in the same token (iBGT), the function correctly unstakes their original deposit but:

- The rewards are credited to the user's Dolomite Margin account balance
- The same tokens are...

Vulnerable code:
function _exit() internal {
    IInfraredVault vault = registry().iBgtStakingVault();

    IInfraredVault.UserReward[] memory rewards = vault.getAllRewardsForUser(address(this));
    vault.exit();

    _handleRewards(rewards);
}

function _handleRewards(IInfraredVault.UserReward[] memory _rewards) internal {
    IIsolationModeVaultFactory factory = IIsolationModeVaultFactory(VAULT_FACTORY());
    for (uint256 i = 0; i < _rewards.length; ++i) {
        if (_rewards[i].amount > 0) {
            if (_rewards[i].token == UNDERLYING_TOKEN()) {
                _setIsDepositSourceThisVault(true);
                factory.depositIntoDolomiteMargin(
                    DEFAULT_ACCOUNT_NUMBER,
                    _rewards[i].amount
                ); //@audit restakes the reward amount
                assert(!isDepositSourceThisVault());
            } else {
                // Handle other tokens...
            }
        }
    }
}

it('should demonstrate that iBGT rewards are re-staked after exit', async () => {
      await testInfraredVault.setRewardTokens([core.tokens.iBgt.address]);
      await core.tokens.iBgt.connect(iBgtWhale).approve(testInfraredVault.address, rewardAmount);
      await testInfraredVault.connect(iBgtWhale).addReward(core.tokens.iBgt.address, rewardAmount);
      await registry.connect(core.governance).ownerSetIBgtStakingVault(testInfraredVault.address);

      await iBgtVault.depositIntoVaultForDolomiteMargin(defaultAccountNumber, amountWei);
      await expectProtocolBalance(core, iBgtVault, defaultAccountNumber, iBgtMarketId, amountWei);

      // Get   the initial staking balance before exit
      const initialStakingBalance = await testInfraredVault.balanceOf(iBgtVault.address);
      expect(initialStakingBalance).to.eq(amountWei);

      // Call exit which should unstake original amount but rewards will be re-staked
      await iBgtVault.exit();

      // Check that staking balance equals reward amount (rewards were re-staked)
      const finalStakingBalance = await testInfraredVault.balanceOf(iBgtVault.address);
      expect(finalStakingBalance).to.eq(rewardAmount, "Staking balance should equal reward amount after exit");

      // Verify original deposit is in the wallet (but not rewards)
      await expectWalletBalance(iBgtVault, core.tokens.iBgt, amountWei);

      // Verify protocol balance now includes both original deposit and rewards
      await expectProtocolBalance(core, iBgtVault, defaultAccountNumber, iBgtMarketId, amountWei.add(rewardAmount));
    });
LOW severity: Missing validation for initialization calldata in `POLIsolationModeWrapperUpgradeableProxy` constructor. **Description:** The `POLIsolationModeWrapperUpgradeableProxy` constructor accepts initialization calldata without performing any validation on its content before executing it via `delegatecall` to the implementation contract. Specifically:

- The constructor does not verify that the `calldata`...

Vulnerable code:
// POLIsolationModeWrapperUpgradeableProxy.sol
constructor(
    address _berachainRewardsRegistry,
    bytes memory _initializationCalldata
) {
    BERACHAIN_REWARDS_REGISTRY = IBerachainRewardsRegistry(_berachainRewardsRegistry);
    Address.functionDelegateCall(
        implementation(),
        _initializationCalldata,
        "POLIsolationModeWrapperProxy: Initialization failed"
    );
}

constructor(
    address _berachainRewardsRegistry,
    bytes memory _initializationCalldata
) {
    BERACHAIN_REWARDS_REGISTRY = IBerachainRewardsRegistry(_berachainRewardsRegistry);

    // Validate function selector is initialize(address)
    require(
        _initializationCalldata.length == 36 &&
        bytes4(_initializationCalldata[0:4]) == bytes4(keccak256("initialize(address)")),
        "Invalid initialization function"
    );

     // Decode and validate the vaultFactory address is non-zero
    address vaultFactory = abi.decode(_initializationCalldata[4:], (address));
    require(vaultFactory != address(0), "Zero vault factory address");

    Address.functionDelegateCall(
        implementation(),
        _initializationCalldata,
        "POLIsolationModeWrapperProxy: Initialization failed"
    );
}
LOW severity: Reward loss risk when transitioning between reward vault types. **Description:** When a user's default reward vault type for an asset is changed (e.g., from NATIVE or BGTM to INFRARED) in the `InfraredBGTMetaVault` contract, current logic attempts to claim outstanding rewards before switching the type. However, the implementation fails to retrieve rewards from.....

Vulnerable code:
function _getReward(address _asset) internal {
    IBerachainRewardsRegistry.RewardVaultType rewardVaultType = IBerachainRewardsRegistry.RewardVaultType.INFRARED;
    IInfraredVault rewardVault = IInfraredVault(REGISTRY().rewardVault(
        _asset,
        rewardVaultType  // Always uses INFRARED, ignoring user's current type
    ));
    // ... claim rewards logic ...
}

assert(getStakedBalanceByAssetAndType(_asset, currentType) == 0);
LOW severity: Rewards in iBGT cannot be redeemed when infrared vault staking is paused. **Description:** Current reward handling mechanism enforces automatic reinvestment of iBGT rewards back into the staking pool, without providing an alternative when staking is disabled.

When the Infrared protocol pauses staking (which can happen for various reasons such as security emergencies or.....

Vulnerable code:
function _handleRewards(IInfraredVault.UserReward[] memory _rewards) internal {
    IIsolationModeVaultFactory factory = IIsolationModeVaultFactory(VAULT_FACTORY());
    for (uint256 i = 0; i < _rewards.length; ++i) {
        if (_rewards[i].amount > 0) {
            if (_rewards[i].token == UNDERLYING_TOKEN()) {
                _setIsDepositSourceThisVault(true);
                factory.depositIntoDolomiteMargin(
                    DEFAULT_ACCOUNT_NUMBER,
                    _rewards[i].amount
                );
                assert(!isDepositSourceThisVault());
            } else {
                // ... handle other token types ...
            }
        }
    }
}

/// @inheritdoc IInfraredVault
function pauseStaking() external onlyInfrared {
    if (paused()) return;
    _pause();
}

function stake(uint256 amount) external whenNotPaused {
     // code
HIGH severity: Missing source validation in CCIP message handling. **Description:** YieldFi integrates with Chainlink CCIP to facilitate cross-chain transfers of its yield tokens (`YToken`). This functionality is handled by the `BridgeCCIP` contract, which manages token accounting for these transfers.

However, in the...

Vulnerable code:
/// handle a received message
function _ccipReceive(Client.Any2EVMMessage memory any2EvmMessage) internal override {
    bytes memory message = abi.decode(any2EvmMessage.data, (bytes)); // abi-decoding of the sent text
    BridgeSendPayload memory payload = Codec.decodeBridgeSendPayload(message);
    bytes32 _hash = keccak256(abi.encode(message, any2EvmMessage.messageId));
    require(!processedMessages[_hash], "processed");

    processedMessages[_hash] = true;

    require(payload.amount > 0, "!amount");

    ...
}

mapping(uint64 sourceChain => mapping(address peer => bool allowed)) public allowedPeers;
...
function _ccipReceive(
    Client.Any2EVMMessage memory any2EvmMessage
) internal override {
    address sender = abi.decode(any2EvmMessage.sender, (address));
    require(allowedPeers[any2EvmMessage.sourceChainSelector][sender],"allowed");
    ...