Farseen0/scar-pairs huggingface dataset:
HIGH severity: Hypervisor collateral redemption can cause vaults to become undercollateralized due to slippage. **Description:** When redeeming Hypervisor collateral, `SmartVaultV4::autoRedemption` performs a call to `SmartVaultYieldManager::quickWithdraw`:

This invokes `SmartVaultYieldManager::_withdrawOtherDeposit` to withdraw the underlying collateral without applying the additional protocol withdrawal...

Vulnerable code:
if (_hypervisor != address(0)) {
    address _yieldManager = ISmartVaultManager(manager).yieldManager();
    IERC20(_hypervisor).safeIncreaseAllowance(_yieldManager, getAssetBalance(_hypervisor));
    _withdrawn = ISmartVaultYieldManager(_yieldManager).quickWithdraw(_hypervisor, _collateralToken);
    IERC20(_hypervisor).forceApprove(_yieldManager, 0);
}

function quickWithdraw(address _hypervisor, address _token) external returns (uint256 _withdrawn) {
    IERC20(_hypervisor).safeTransferFrom(msg.sender, address(this), IERC20(_hypervisor).balanceOf(msg.sender));
    _withdrawOtherDeposit(_hypervisor, _token);
    uint256 _withdrawn = _thisBalanceOf(_token);
    IERC20(_token).safeTransfer(msg.sender, _withdrawn);
}

function redeposit(uint256 _withdrawn, uint256 _collateralBalance, address _hypervisor, address _collateralToken)
    private
{
    uint256 _redeposit = _withdrawn > _collateralBalance ? _collateralBalance : _withdrawn;
    address _yieldManager = ISmartVaultManager(manager).yieldManager();
    IERC20(_collateralToken).safeIncreaseAllowance(_yieldManager, _redeposit);
    ISmartVaultYieldManager(_yieldManager).quickDeposit(_hypervisor, _collateralToken, _redeposit);
    IERC20(_collateralToken).forceApprove(_yieldManager, 0);
}
HIGH severity: Vaults can be made erroneously liquidatable due to incorrect swap path. **Description:** When auto redemption is executed, the `Quoter::quoteExactOutput` and `Quoter::quoteExactInput` functions of the [Uniswap v3 view-only quoter](https://github.com/Uniswap/view-quoter-v3/tree/master) are invoked for legacy and non-legacy vaults respectively. Given that these calls do.....

Vulnerable code:
_amountOut = ISwapRouter(_swapRouterAddress).exactInput(
    ISwapRouter.ExactInputParams({
        path: _swapPath,
        recipient: address(this),
        deadline: block.timestamp,
        amountIn: _collateralAmount,
        // minimum amount out should be at least usd value of collateral being swapped in
        amountOutMinimum: calculator.tokenToUSD(
            ITokenManager(ISmartVaultManager(manager).tokenManager()).getTokenIfExists(_collateralAddr),
            _collateralAmount
        )
    })
);

uint256 _usdsBalance = USDs.balanceOf(address(this));
minted -= _usdsBalance;
USDs.burn(address(this), _usdsBalance);
HIGH severity: `AutoRedemption` mappings are not and can never be populated. **Description:** The following mappings are declared within the `AutoRedemption` contract:

However, they are never populated and there are no public functions capable of doing so either.

When a request is triggered, `lastRequestId` is assigned a non-zero identifier:

But the execution of...

Vulnerable code:
mapping(address => address) hypervisorCollaterals;
mapping(address => bytes) swapPaths;

lastRequestId = _sendRequest(req.encodeCBOR(), subscriptionID, MAX_REQ_GAS, donID);

bytes memory _collateralToUSDCPath = swapPaths[_token];
...
lastRequestId = bytes32(0);
HIGH severity: Auto redemption logic can be abused by an attacker due to insufficient access control. **Description:** `AutoRedemption::performUpkeep` is exposed for use by the Chainlink Automation DON when upkeep is required:

However, there is an absence of access control that allows the function to be called by an address. Since `lastRequestId` is reset to `bytes32(0)` at the end of...

Vulnerable code:
function performUpkeep(bytes calldata performData) external {
    if (lastRequestId == bytes32(0)) {
        triggerRequest();
    }
}

uint256 _usdsBalance = USDs.balanceOf(address(this));
minted -= _usdsBalance;
HIGH severity: Auto redemption functionality broken by incorrect address passed to `SmartVaultV4::autoRedemption`. **Description:** `SmartVaultV4::autoRedemption` has the following signature:

However the invocation in `AutoRedemption::fulfillRequest` incorrectly passes the vault address in place of the swap router address:

**Impact:** Auto redemption functionality will become completely broken when...

Vulnerable code:
function autoRedemption(
    address _swapRouterAddress,
    address _quoterAddress,
    address _collateralToken,
    bytes memory _swapPath,
    uint256 _USDCTargetAmount,
    address _hypervisor
)

IRedeemable(_smartVault).autoRedemption(
    _smartVault, quoter, _token, _collateralToUSDCPath, _USDsTargetAmount, _hypervisor
);
MEDIUM severity: Automation and redemption could be artificially manipulated due to use of instantaneous `sqrtPriceX96`. **Description:** `AutoRedemption::checkUpkeep` is queried every block by the Chainlink Automation DON:

However, use of `sqrtPriceX96` is problematic since it is based on the instantaneous pool reserves which can be manipulated to force upkeep into triggering even if it is not required....

Vulnerable code:
function checkUpkeep(bytes calldata checkData) external returns (bool upkeepNeeded, bytes memory performData) {
    (uint160 sqrtPriceX96,,,,,,) = pool.slot0();
    upkeepNeeded = sqrtPriceX96 <= triggerPrice;
}

function calculateUSDsToTargetPrice() private view returns (uint256 _usdc) {
    int24 _spacing = pool.tickSpacing();
    (uint160 _sqrtPriceX96, int24 _tick,,,,,) = pool.slot0();
    int24 _upperTick = _tick / _spacing * _spacing;
    int24 _lowerTick = _upperTick - _spacing;
    uint128 _liquidity = pool.liquidity();
    ...
}

if (_USDsTargetAmount > _vaultData.status.minted) _USDsTargetAmount = _vaultData.status.minted;
MEDIUM severity: `USDs` redemption calculation can be manipulated due to unsafe signed-unsigned cast. **Description:** The following logic is executed when calculating the `USDs` redemption amount, after the tick range has been advanced beyond that of the current tick:

The `_liquidityNet` variable represents the net active liquidity delta when crossing between tick ranges, so this is considered...

Vulnerable code:
} else {
    (, int128 _liquidityNet,,,,,,) = pool.ticks(_lowerTick);
    _liquidity += uint128(_liquidityNet);
    (_amount0,) = LiquidityAmounts.getAmountsForLiquidity(
        _sqrtPriceX96,
        TickMath.getSqrtRatioAtTick(_lowerTick),
        TickMath.getSqrtRatioAtTick(_upperTick),
        _liquidity
    );
}

if (_USDsTargetAmount > _vaultData.status.minted) _USDsTargetAmount = _vaultData.status.minted;
MEDIUM severity: Incorrect collateral quote amounts due to use of incorrect swap path. **Description:** Within `AutoRedemption::legacyAutoRedemption`, the amount of input collateral required is calculated as:

This quote is incorrect as it uses the `collateral -> USDC` swap path for calculation of a target `USDs` amount. Since `USDs` is expected to be below peg for this logic to be...

Vulnerable code:
(uint256 _approxAmountInRequired,,,) =
IQuoter(quoter).quoteExactOutput(_collateralToUSDCPath, _USDsTargetAmount);
uint256 _amountIn = _approxAmountInRequired > _collateralBalance ? _collateralBalance : _approxAmountInRequired;

(uint256 _quoteAmountOut,,,) = IQuoter(_quoterAddress).quoteExactInput(_swapPath, _collateralBalance);
return _quoteAmountOut > _USDCTargetAmount
    ? _collateralBalance * _USDCTargetAmount / _quoteAmountOut
    : _collateralBalance;
MEDIUM severity: Auto redemption will not function due to slippage misconfiguration. **Description:** Both legacy and non-legacy vaults perform a call to `ISwapRouter::exactInput` when attempting to swap collateral to `USDs`, with the `amountOutMinimum` parameter being specified as the USD value of the collateral being swapped:

However, this fails to account for slippage incurred.....

Vulnerable code:
calculator.tokenToUSD(
    ITokenManager(ISmartVaultManager(manager).tokenManager()).getTokenIfExists(_collateralAddr),
    _collateralAmount
)
LOW severity: Concentrated liquidity tick logic is incorrect. **Description:** `AutoRedemption::calculateUSDsToTargetPrice` performs the following concentrated liquidity tick calculations to determine the current tick range corresponding to a given price:

However, due to the behavior of signed integers in Solidity rounding _up_ to zero rather than the next...

Vulnerable code:
int24 _spacing = pool.tickSpacing();
(uint160 _sqrtPriceX96, int24 _tick,,,,,) = pool.slot0();
int24 _upperTick = _tick / _spacing * _spacing;
int24 _lowerTick = _upperTick - _spacing;

while (TickMath.getSqrtRatioAtTick(_lowerTick) < TARGET_PRICE) {
    uint256 _amount0;
    if (_tick > _lowerTick && _tick < _upperTick) {
        (_amount0,) = LiquidityAmounts.getAmountsForLiquidity(
            _sqrtPriceX96,
            TickMath.getSqrtRatioAtTick(_lowerTick),
            TickMath.getSqrtRatioAtTick(_upperTick),
            _liquidity
        );
    } else {
        (, int128 _liquidityNet,,,,,,) = pool.ticks(_lowerTick);
        _liquidity += uint128(_liquidityNet);
        (_amount0,) = LiquidityAmounts.getAmountsForLiquidity(
            _sqrtPriceX96,
            TickMath.getSqrtRatioAtTick(_lowerTick),
            TickMath.getSqrtRatioAtTick(_upperTick),
            _liquidity
        );
    }
    _usdc += _amount0;
    _lowerTick += _spacing;
    _upperTick += _spacing;
}
HIGH severity: Unused function argument in `SmartVaultYieldManager::quickDeposit` should be removed. **Description:** The `SmartVaultYieldManager::quickDeposit` function has the following signature:

However, the `_hypervisor` argument is never used and so can be removed.

**The Standard DAO:** Fixed by commit...

Vulnerable code:
function quickDeposit(address _hypervisor, address _collateralToken, uint256 _deposit)
HIGH severity: The vault limit condition should not be checked for `address(0)`. **Description:** The virtual function `ERC721Upgradeable::_update` has been overridden within `SmartVaultManagerV6` as follows:

However, the `userVaultLimit` validation should not be performed for `address(0)` otherwise it will become impossible to burn more than this number of tokens....

Vulnerable code:
function _update(address _to, uint256 _tokenID, address _auth) internal virtual override returns (address) {
    address _from = super._update(_to, _tokenID, _auth);
    require(vaultIDs(_to).length < userVaultLimit, "err-vault-limit");
    smartVaultIndex.transferTokenId(_from, _to, _tokenID);
    if (address(_from) != address(0)) ISmartVault(smartVaultIndex.getVaultAddress(_tokenID)).setOwner(_to);
    emit VaultTransferred(_tokenID, _from, _to);
    return _from;
}

function transferTokenId(address _from, address _to, uint256 _tokenId) external onlyManager {
    removeTokenId(_from, _tokenId);
    tokenIds[_to].push(_tokenId);
}
LOW severity: Lack of validation for default admin revocation during initialization. **Description:** In `ShibuyaToken::initializeV3()`, the contract attempts to revoke `DEFAULT_ADMIN_ROLE` from the provided `defaultAdmin` parameter. Based on the deployed contract on testnet and team communication, the intention was to remove the role from the original default admin address during.....

Vulnerable code:
Shibuya.sol
36:         // this is to make sure that the default admin is the owner and remove the default admin role from the deployer
37:         // so this means that the deployer will not have any role in the contract
38:         // the the DEFAULT_ADMIN_ROLE will not have any role in the contract
39:         // and the owner will be performing the role of the admin
40:         _revokeRole(DEFAULT_ADMIN_ROLE, defaultAdmin);
HIGH severity: Unrestricted reSDL token deposits with privileged withdrawals can lead to accidental loss of reSDL tokens. **Description:** The `StakingProxy` contract implements ERC721 receiver functionality allowing it to receive reSDL tokens (ERC721) from any address. However, only the contract owner has the ability to withdraw these tokens.

This creates a risk where user owned reSDL tokens can get stuck if sent...

Vulnerable code:
// ----> @audit Anyone can transfer reSDL tokens to the proxy
function onERC721Received(address, address, uint256, bytes calldata) external returns (bytes4) {
    return this.onERC721Received.selector;
}

// ----> @audit Only owner can withdraw reSDL tokens
function withdrawRESDLToken(uint256 _tokenId, address _receiver) external onlyOwner {
    if (sdlPool.ownerOf(_tokenId) != address(this)) revert InvalidTokenId();
    IERC721(address(sdlPool)).safeTransferFrom(address(this), _receiver, _tokenId);
}
LOW severity: Storage collision risk in UUPS upgradeable `StakingProxy` due to missing storage gap. **Description:** `StakingProxy` contract inherits from `UUPSUpgradeable` and `OwnableUpgradeable` but does not implement storage gaps to protect against storage collisions during upgrades.

`StakingProxy` is intended to be used by third parties/DAOs. It is possible that this contract gets...

Vulnerable code:
contract StakingProxy is UUPSUpgradeable, OwnableUpgradeable {
    // address of asset token
    IERC20Upgradeable public token;
    // address of liquid staking token
    IStakingPool public lst;
    // address of priority pool
    IPriorityPool public priorityPool;
    // address of withdrawal pool
    IWithdrawalPool public withdrawalPool;
    // address of SDL pool
    ISDLPool public sdlPool;
    // address authorized to deposit/withdraw asset tokens
    address public staker; // ---> @audit missing storage slots
}
HIGH severity: Reward claiming fails for Berachain RewardVaults due to incorrect interface. **Description:** D2 plans to deploy on the newly launched Berachain, which operates on a novel [Proof-of-Liquidity](https://docs.berachain.com/learn/what-is-proof-of-liquidity) model. On Berachain, liquidity providers can stake their LP tokens in [Reward...

Vulnerable code:
function bera_vault_get_reward(address token) external onlyRole(EXECUTOR_ROLE) nonReentrant {
    IVault(vaultFactory.getVault(token)).getReward(address(this));
}

function getReward(address account) external;

function getReward(
    address account,
    address recipient
)
HIGH severity: Lack of slippage protection allows exploitation of Pendle trades. **Description:** The [`Pendle_Module`](https://github.com/d2sd2s/d2-contracts/blob/c2fc257605ebc725525028a5c17f30c74202010b/contracts/modules/Pendle.sol) is designed to interact with the Pendle protocol. [Pendle](https://pendle.finance/) is a permissionless yield-trading protocol that enables...

Vulnerable code:
router.addLiquiditySingleToken(
      address(this),
      address(market),
      0, // @audit `minLpOut` hardcoded to 0
      approxParams,
      input,
      limitOrderData
  );

IPRouter.TokenOutput memory output = IPRouter.TokenOutput({
      tokenOut: ast,
      minTokenOut: 0, // @audit `minTokenOut` hardcoded to 0
      tokenRedeemSy: ast,
      pendleSwap: address(0),
      swapData: swapData
  });

IPRouter.TokenOutput memory output = IPRouter.TokenOutput({
      tokenOut: ast,
      minTokenOut: 0, // @audit `minTokenOut` hardcoded to 0
      tokenRedeemSy: ast,
      pendleSwap: address(0),
      swapData: swapData
  });
HIGH severity: Unsafe token transfers in `VaultV3` can lead to state inconsistencies. **Description:** The `VaultV3` contract doesn't validate the success of token transfers in both `custodyFunds()` and `returnFunds()` functions. State changes occur before or regardless of transfer success, which can lead to vault state inconsistencies.

**Impact:** Even with a trusted trader,...

Vulnerable code:
function custodyFunds() external onlyTrader notCustodied duringEpoch returns (uint256) {
    uint256 amount = totalAssets();
    require(amount > 0, "!amount");
    custodied = true;
    custodiedAmount = amount;
    IERC20(asset()).transfer(trader, amount); // No success validation
    emit FundsCustodied(epochId, amount);
    return amount;
}

function returnFunds(uint256 _amount) external onlyTrader {
    require(custodied, "!custody");
    require(_amount > 0, "!amount");
    IERC20(asset()).transferFrom(trader, address(this), _amount); // No success validation
    epoch.epochEnd = uint80(block.timestamp);
    custodied = false;
    emit FundsReturned(currentEpoch, _amount);
}

import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

contract VaultV3 {
    using SafeERC20 for IERC20;

    function custodyFunds() external onlyTrader notCustodied duringEpoch returns (uint256) {
        uint256 amount = totalAssets();
        require(amount > 0, "!amount");
        custodied = true;
        custodiedAmount = amount;
        IERC20(asset()).safeTransfer(trader, amount);
        emit FundsCustodied(epochId, amount);
        return amount;
    }

    function returnFunds(uint256 _amount) external onlyTrader {
        require(custodied, "!custody");
        require(_amount > 0, "!amount");
        epoch.epochEnd = uint80(block.timestamp);
        custodied = false;
        IERC20(asset()).safeTransferFrom(trader, address(this), _amount);
        emit FundsReturned(currentEpoch, _amount);
    }
}
HIGH severity: Incorrect `chainid` prevents correct Strategy deployment on Berachain. **Description:** The Strategy contract includes a special configuration specifying which facets should be used for different chains. However, the `chainid` assigned to Berachain is incorrect, as seen in ...

Vulnerable code:
} else if (block.chainid == 80000) { // @audit Berachain id is 80094

- } else if (block.chainid == 80000) {
+ } else if (block.chainid == 80094) {
HIGH severity: Incorrect interface for `ExchangeRouter::updateOrder` prevents order updates in GMX V2. **Description:** There is an issue in the integration with the GMX V2 trading platform, specifically in the [`GMX_Module::gmxv2_update`](https://github.com/d2sd2s/d2-contracts/blob/c2fc257605ebc725525028a5c17f30c74202010b/contracts/modules/GMXV2.sol#L140-L148) function:

The issue arises from an...

Vulnerable code:
function gmxv2_update(
    bytes32 key,
    uint256 sizeDeltaUsd,
    uint256 acceptablePrice,
    uint256 triggerPrice,
    uint256 minOutputAmount
) external onlyRole(EXECUTOR_ROLE) nonReentrant {
    exchangeRouter.updateOrder(key, sizeDeltaUsd, acceptablePrice, triggerPrice, minOutputAmount);
}

function updateOrder(
    bytes32 key,
    uint256 sizeDeltaUsd,
    uint256 acceptablePrice,
    uint256 triggerPrice,
    uint256 minOutputAmount
) external payable;

function updateOrder(
    bytes32 key,
    uint256 sizeDeltaUsd,
    uint256 acceptablePrice,
    uint256 triggerPrice,
    uint256 minOutputAmount,
    uint256 validFromTime,
    bool autoCancel
) external payable nonReentrant {
HIGH severity: `Bera_Module` fails to stake in `BGTStaker` due to missing Boost integration. **Description:** The D2 Berachain integration includes two functions, [`Bera_Module::bera_bgt_stake`](https://github.com/d2sd2s/d2-contracts/blob/c2fc257605ebc725525028a5c17f30c74202010b/contracts/modules/Bera.sol#L54-L56) and...

Vulnerable code:
/// @dev Throws if called by any account other than BGT contract.
modifier onlyBGT() {
    if (msg.sender != address(stakeToken)) NotBGT.selector.revertWith();
    _;
}
...

function stake(address account, uint256 amount) external onlyBGT {
    _stake(account, amount);
}
HIGH severity: `Bera_Module::bera_kodiakv3_swap` broken due to `deadline` parameter. **Description:** The function `Bera_Module::bera_kodiakv3_swap` calls Kodiak's `SwapRouter02::exactInput` (which is similar to Uniswap V3) to execute a swap:

However, the `deadline` parameter is included in the function call but is not present in the...

Vulnerable code:
function bera_kodiakv3_swap(address token, uint amount, uint amountMin, bytes calldata path) external onlyRole(EXECUTOR_ROLE) nonReentrant {
    validateToken(token);
    IERC20(token).approve(address(kodiakv3swap), amount);
    kodiakv3swap.exactInput(IKodiakV3.ExactInputParams({
        path: path,
        recipient: address(this),
        deadline: type(uint256).max,
        amountIn: amount,
        amountOutMinimum: amountMin
    }));
}

struct ExactInputParams {
    bytes path;
    address recipient;
    uint256 amountIn;
    uint256 amountOutMinimum;
}

function test_bera_kodiakv3_swap() public {
    deal(WBERA_ADDRESS, address(strategy), 1e18);

    bytes memory path = abi.encodePacked(WBERA_ADDRESS, uint24(3000), HONEY_ADDRESS);

    vm.prank(EXECUTOR);
    bera.bera_kodiakv3_swap(
        WBERA_ADDRESS,
        1e18,
        0,
        path
    );

    assertGt(HONEY.balanceOf(address(strategy)),0);
}
HIGH severity: Kodiak swap functions do not check if output token is approved, risking stuck assets. **Description:** In the swap implementations within `Inch_Module`, both the input token and output token are validated to ensure they are approved before executing a swap. This validation is present in...

Vulnerable code:
function test_bera_kodiakv2_swap_to_non_allowed_token() public {
    deal(WBERA_ADDRESS, address(strategy), 1e18);

    address[] memory path = new address[](2);
    path[0] = WBERA_ADDRESS;
    path[1] = RAMEN_TOKEN_ADDRESS; // not allowed

    vm.startPrank(EXECUTOR);
    bera.bera_kodiakv2_swap(
        WBERA_ADDRESS,
        1e18,
        0,
        path
    );

    uint256 ramenBalance = ERC20(RAMEN_TOKEN_ADDRESS).balanceOf(address(strategy));
    assertGt(ramenBalance,0);

    path[0] = RAMEN_TOKEN_ADDRESS;
    path[1] = WBERA_ADDRESS;

    // token cannot be swapped back
    vm.expectRevert("Invalid token");
    bera.bera_kodiakv2_swap(
        RAMEN_TOKEN_ADDRESS,
        ramenBalance,
        0,
        path
    );
    vm.stopPrank();

    // balance still left in contract
    assertEq(ERC20(RAMEN_TOKEN_ADDRESS).balanceOf(address(strategy)), ramenBalance);
}
HIGH severity: Compromised trader account can block admin from revoking access. **Description:** The trading strategy contract for D2 defines two roles: `ADMIN_ROLE` and `EXECUTOR_ROLE`. The `EXECUTOR_ROLE` is intended for the active party responsible for executing trades on behalf of stakers, while the `ADMIN_ROLE` is a more passive role, primarily reserved for emergency...

Vulnerable code:
s.grantRole(ADMIN_ROLE, args._owner);
s.grantRole(EXECUTOR_ROLE, args._owner);
s.grantRole(ADMIN_ROLE, args._trader); // @audit admin given to trader as well
s.grantRole(EXECUTOR_ROLE, args._trader);

s.grantRole(ADMIN_ROLE, args._owner);
  s.grantRole(EXECUTOR_ROLE, args._owner);
- s.grantRole(ADMIN_ROLE, args._trader);
  s.grantRole(EXECUTOR_ROLE, args._trader);
LOW severity: Improper deadline handling. **Description:** The Bera module shows improper handling of transaction deadlines in DEX operations:

- KodiakV2/V3 operations use `type(uint256).max` as deadline:

- OogaBooga and KodiakV3 (see issue _7.3.6 `Bera_Module::bera_kodiakv3_swap` broken due to `deadline` parameter_) swaps lack deadline.....

Vulnerable code:
// Bera.sol
function bera_kodiakv2_add(...) {
    kodiakv2.addLiquidity(..., type(uint256).max);
}

function bera_kodiakv2_swap(address token, uint amount, uint amountMin, address[] calldata path) external onlyRole(EXECUTOR_ROLE) nonReentrant {
    ...
    kodiakv2.swapExactTokensForTokensSupportingFeeOnTransferTokens(
        ...
        type(uint256).max
    );
}

function bera_kodiakv3_increase(...) {
    kodiakv3.increaseLiquidity(IKodiakV3.IncreaseLiquidityParams({
        ...
        deadline: type(uint256).max
    }));
}

function bera_oogabooga_swap(...) {
    oogaBooga.swap(tokenInfo, pathDefinition, executor, referralCode);
}

uint256 private constant MAX_DEADLINE_WINDOW = 30 minutes;

function bera_kodiakv2_add(...) {
    kodiakv2.addLiquidity(..., block.timestamp + MAX_DEADLINE_WINDOW);
}