Owner can takeover funds meant for distribution

Lines of code Vulnerability details Impact By calling sweep function at correct moment, Owner can transfer more than required tokenOut token which were meant to be distributed to users. Proof of Concept 1. Observe the sweep function function sweepaddress token external gacPausable nonReentrant...

Initial StakedCitadel deposit with amount=1 wei causes very expensive share price leading to precision errors and loss of funds

Lines of code Vulnerability details Impact The first depositor into StakedCitadel is able to maliciously manipulate the share price by depositing the lowest possible amount 1 wei and then artificially blowing up the StakedCitadel Citadel token balance. Following depositors will loose their...

IndexLogic: An attacker can mint tokens for himself using assets deposited by other users

Lines of code Vulnerability details Impact In the mint function of the IndexLogic contract, users are required to transfer assets to vToken in advance, and then call the mint function to mint tokens. The attacker can monitor the asset balance in the vToken contract. When the balance is greater th...

Users wont be able to withdraw staked tokens since vesting function is called differently in interface and implementation

Lines of code Vulnerability details Impact Users can't withdraw their funds Proof of Concept When withdrawing in staking the contract will send funds to vesting using setupVesting. This is defined in the vesting interface. On the other hand, the vesting implementation calls this function vest so...

withdrawal amount might be wrong

Lines of code Vulnerability details r is the user's part of the contract balance, but is supposed to be the user's part of the total funds, including the strategy funds. therefore the check at line 816 will always return false because the user's part of the contract balance is smaller than the...

Funding.deposit() doesn't work if there is no discount set

Lines of code Vulnerability details Impact The Funding contract's deposit function uses the getAmountOut function to determine how many citadel tokens the user should receive for their deposit. But, if no discount is set, the function always returns 0. Now the deposit function tries to deposit 0...

Race between governance and strategist on other token earned

Lines of code Vulnerability details Impact There is a race between the strategist and the governance to report other tokens earned by the strategy. Indeed the strategist can trigger the function 1 by calling the strategy while the governance can call 2. Both these functions can report earn tokens...

Function name doesn't match the interface

Lines of code Vulnerability details Impact StakedCitadelVester contract doesn't support the IVesting interface, so the withdrawal of Citadel from the StakedCitadel contract will be impossible because the IVestingvesting.setupVestingmsg.sender, amount, block.timestamp; call will revert because the...

If there is no discount then buying citadel in funding will always return 0 citadel

Lines of code Vulnerability details Impact Users are unable to buy citadel in funding if the discount is 0. Proof of Concept In the funding contract if the discount is 0 then getAmountOut will always return 0 and users won't be able to use funding to buy citadel. If discount is 0 then the if is...

_mintSharesFor doesn’t check whether the pool is zero.

Lines of code Vulnerability details Impact In StakedCitadel.sol/mintSharesFor, it doesn’t check whether the pool is equal to zero. If pool == 0 and totalSupply != 0, mintSharesFor will revert. And In depositFor, it uses balance as pool. In consequence, when the CTDL balance of StakedCitadel is ze...

User funds stuck - buy works even after sale ends

Lines of code Vulnerability details Impact Buy function is not checking finalized value. Under specific scenario, Buy function will work even when sale has ended and finalize function has been called. If user buys any token at this moment then he wont be able to claim and will lose funds Proof of...

Staked Citadel function balance and reportHarvest

Lines of code Vulnerability details Impact In the StakedCitadel...

Funding contract fails without discount

Lines of code Vulnerability details Impact The deposit function of Funding.sol calls getAmountOut to determine the amount of citadel the user receives in exchange for an asset. The getAmountOut function contains a bug that results in the return being 0 if funding.discount is set to 0. In the...

StakedCitadel should implement EIP-4626

Lines of code Vulnerability details StakedCitadel should implement EIP-4626 StakedCitadel could be seen as an investment vault and therefore team should consider implementing the recent EIP-4626 standard, as it could enable a lot of potential integrations and would avoid having to deploy wrappers...

Should check return data from chainlink aggregators

Lines of code Vulnerability details Impact The refreshedAssetPerBaseInUQ function in the contract ChainlinkPriceOracle.sol fetches the asset price from a Chainlink aggregator using the latestRoundData function. However, there are no checks on roundID nor timeStamp, resulting in stale prices. The...

latestRoundData might return stale

Lines of code Vulnerability details Impact The conctract uses latestRoundData but there is no check if the returned value is stale data or not Proof of Concept code-423n4/2021-12-perennial-findings24 code-423n4/2021-06-tracer-findings73 Tools Used Manual analysis Recommended Mitigation Steps...

Incorrect interface for StakedCitadelVester causes withdrawals to revert

Lines of code Vulnerability details The IVesting interface contains a setupVesting function: IVesting4 interface IVesting function setupVesting address recipient, uint256 amount, uint256 unlockBegin external; This function is called to send withdrawals to the vesting contract in...

Governor Infinitely Extend To Vesting Period

Lines of code Vulnerability details Impact During the review of StakedCitadelVester contract, It has been noticed governor role can extend vesting period indefinitely. There is no upper bound defined in the vesting period. Although, It is dependent on the governor, If governor private key is...

TECH_OPERATIONS_ROLE is not initialized properly

Lines of code Vulnerability details Impact setGuestlist and setTokenInLimit are unavailable for use during KnightingRound to use due to a mistake in initializing roles. Proof of Concept The role TECHOPERATIONSROLE, created in L30 of GlobalAccessControl.sol , is not initialized in the initialize t...

latestRoundData data insufficiently validated

Lines of code Vulnerability details Impact The data returned by the Chainlink latestRoundData function may be stale. There should be checks applied on the data received from Chainlink to validate that it is not stale. Proof of Concept The ChainlinkPriceOracle contract has these two lines , int...

Chainlink oracle might return stale data

Lines of code Vulnerability details Impact Oracle might return stale data for basePrice and quotePrice. Proof of Concept refreshedAssetPerBaseInUQ in ChainlinkPriceOracle.sol does not check if the data from Chainlink is fresh . If there is a problem with the Chainlink oracle, this contract may be...

Improper Validation Of Chainlink's latestRoundData Function

Lines of code Vulnerability details Impact When using Chainlink Price feeds, it is important to ensure the price feed data was updated recently. While getting started with chainlink requires just one line of code, it is best to add additional checks for "in production" environments. Here,...

missing a check

Lines of code Vulnerability details Impact in citadelMinter.setCitadelDistributionSplit there is a require check that check the sum of the propvalues must be 10000 bps and POLICYOPERATIONSROLE can set 2 of the variable to 0 and 1 to 10000 it is better to make a min value to the variables and chec...

Wrong calculation for yVault price per share if decimals != 18

Lines of code Vulnerability details The yVault.getPricePerFullShare function calculates the price per share by multiplying with 1e18 token decimals with the assumption that the underlying token always has 18 decimals. yVault has the same amount of decimals as it's underlying token see...

New YVault depositors can be attacked by depressing share decimals

Lines of code Vulnerability details Impact An attacker can become the first depositor for a recently created YVault contract, providing a tiny amount of token by calling deposit1 raw values here, 1 is 1 wei, 1e18 is 1 token if it is 18 decimals. Then the attacker can directly transfer, for exampl...

Update initializer modifier to prevent reentrancy during initialization

Lines of code Vulnerability details Impact The solution uses: "@openzeppelin/contracts": "^4.0.0", "@openzeppelin/contracts-upgradeable": "^4.3.2", These dependencies have a known high severity vulnerability: Which makes these contracts vulnerable: contracts/helpers/CryptoPunksHelper.sol: 19:...

Use of deprecated Chainlink API

Lines of code Vulnerability details Impact The contract uses Chainlink’s deprecated API latestAnswer. Such functions might suddenly stop working if Chainlink stopped supporting deprecated APIs. Impact: Deprecated API stops working. Prices cannot be obtained. Protocol stops and contracts have to b...

!_account.isContract() can be bypassed

Lines of code Vulnerability details Impact A contract in construction can bypass isContract to call deposit and withdraw functions in vaults/yVault/yVault.sol, farming/LPFarming.sol, and farming/yVaultLPFarming.sol. Also, Block contracts may cause DoS if someone uses multisig contracts as a calle...

Uncontrolled usage of Chainlink API for core price retrieval

Lines of code Vulnerability details Impact Chainlink's latestAnswer usage can yield stale price information, which is crucial for borrowing and liquidation. latestAnswer is having less ways to be controlled compared to latestRoundData, which is advised for price sensitive operations. Staling pric...

[WP-H13] LockPosition can be overwritten by new lockFor() which leads to user's funds loss

Lines of code Vulnerability details function lockFor address account, uint256 nftIndex, uint256 lockAmount external onlyOwner nonReentrant jpeg.safeTransferFromaccount, addressthis, lockAmount; positionsnftIndex = LockPosition owner: account, unlockAt: block.timestamp + lockTime, lockAmount:...

Early Depositor can steal funds from subsequent depositors

Lines of code Vulnerability details Impact An early depositor to yVault.sol, preferably the first to deposit, will have the ability to steal funds from subsequent user deposits. The malicious user is able to do this by directly transferring tokens to either the yVault or Controller contracts...

Oracle Prices Used Could Be Stale Or Manipulated due to latestAnswer() call

Lines of code Vulnerability details Impact In the various calls to the Chainlink oracle, the deprecated API function latestAnswer is used. This approach is vulnerable to price manipulation and stale prices according to the Chainlink documentation. This vulnerability was marked as Medium severity ...

[WP-H22] Bad debts should not continue to accrue interest

Lines of code Vulnerability details uint256 debtAmount = getDebtAmountnftIndex; require debtAmount = getLiquidationLimitnftIndex, "positionnotliquidatable" ; // burn all payment stablecoin.burnFrommsg.sender, debtAmount; In the current design/implementation, the liquidator must fully repay the...

use of deprecated chainlink oracle method

Lines of code Vulnerability details Impact latestanswer , this method does not error if no answer has been reached, it will simply return 0, since we have checks in function requireanswer 0, "invalidoracleanswer"; we may not get the latest value of current price which can affect the functionality...

Use of deprecated oracle API in _collateralPriceUsd

Lines of code Vulnerability details Likelihood low, impact high. The Chainlink latestAnswer function included in IAggregatorV3Interface and called in FungibleAssetVaultForDAOcollateralPriceUsd is considered deprecated and no longer included in the Chainlink API documentation. It's considered best...

Initial yVault deposit with amount=1 wei causes very expensive share price leading to precision errors and loss of funds

Lines of code Vulnerability details Impact The first depositor into yVault is able to maliciously manipulate the share price by depositing the lowest possible amount 1 wei and then artificially blowing up the yVault token balance. Following depositors will loose their deposited funds due to...

Use of deprecated Chainlink function latestAnswer

Lines of code Vulnerability details function normalizeAggregatorAnswerIAggregatorV3Interface aggregator internal view returns uint256 int256 answer = aggregator.latestAnswer; uint8 decimals = aggregator.decimals; requireanswer 0, "invalidoracleanswer"; //converts the answer to have 18 decimals...

Use of deprecated Chainlink oracle aggregator function latestAnswer

Lines of code Vulnerability details Impact According to Chainlink's documentation, the latestAnswer function is deprecated. This function does not error if no answer has been reached but returns 0. The function is not present in the latest API reference AggregatorInterfaceV3. Proof of Concept...

[WP-H5] yVault.sol A malicious early user/attacker can manipulate the vault's pricePerShare to take an unfair share of future users' deposits

Lines of code Vulnerability details function deposituint256 amount public noContractmsg.sender requireamount 0, "INVALIDAMOUNT"; uint256 balanceBefore = balance; token.safeTransferFrommsg.sender, addressthis, amount; uint256 supply = totalSupply; uint256 shares; if supply == 0 shares = amount; el...

Use of deprecated oracle API in _normalizeAggregatorAnswer

Lines of code Vulnerability details Use of deprecated oracle API in normalizeAggregatorAnswer Likelihood low, impact high. The Chainlink latestAnswer function included in IAggregatorV3Interface and called in NFTVaultnormalizeAggregatorAnswer is considered deprecated and no longer included in the...

Usage of deprecated Chainlink functions

Lines of code Vulnerability details Impact The Chainlink function latestAnswer is deprecated. Instead, use latestRoundData. As seen in the changelog, Chainlink encourages people to use the latestRoundData function. It's not clear when the support for deprecated functions ends. Here's the same iss...

setDebtInterestApr() doesn't accrue interest before changing it

Lines of code Vulnerability details Impact Wrong interest can be charged if interest is changed without calling accrue Proof of Concept The function setDebtInterestApr updates the interest charged on debt without calling accrue before to compute previous interests. If someone takes debt at 2% and...

Deprecated CHAINLINK API usage

Lines of code Vulnerability details Impact Usage of deprecated chainlink function to get collateral price. Proof of Concept The Chainlink API latestAnswer used in the FungibleAssetVaultForDAO contract is deprecated: This method returns the last value but that value cannot be fully updated. New V3...

JPEGLock.lockFor can relock for specific nft, which overwrites previous lock and cause JPEG loss of the locker

Lines of code Vulnerability details Impact NFTVault contract allows anyone to finalize a pending NFT value proposal by calling the finalizePendingNFTValueETH function. A finalizer must lock an equivalent amount of JPEG tokens to the proposed NFT value in JPEGLock, and can only withdraw those toke...

Usage of deprecated ChainLink API

Lines of code Vulnerability details Impact The Chainlink API latestAnswer function is used in two places but it is deprecated: This API is deprecated. Please see API Reference for the latest Price Feed API. Chainlink Docs The latestAnswer function does not revert if no answer has been reached but...

Use latestRoundData instead latestAnswer of Chainlink aggregators

Lines of code Vulnerability details Impact Use latestAnswer in vaults/NFTVault.sol may get stale ETH price in USD ethPriceUSD, stale value in ETH of the NFT ​​getNFTValueETH, stale JPEG price in USD jpegPriceUSD, and also stale USD price of one unit of collateral asset collateralPriceUsd of...

JPEGLock lockups overwrite previous data

Lines of code Vulnerability details Impact If a user calls NFTVault.finalizePendingNFTValueETH a second time without first calling JPEGLock.unlock to recover their previous lockup, their balance will be overwritten leaving the previous lockup balance unrecoverable. Proof of Concept POC by adding...

finalizePendingNFTValue() May Lock an NFT Value Twice Overwriting The Previous Call

Lines of code Vulnerability details Impact If the same NFT index is locked more than once then the initial locker will lose their funds, they will not be recoverable. The function lockFor does not account for the case where the nftIndex is already in use. If this index is already in use...

Controller.setStrategy tries to withdraw JPEG token with incorrect function strategy.withdraw(address), leading to certain revert and renders setStrategy unuseable

Lines of code Vulnerability details Impact Whenever STRATEGISTs want to switch from currently used strategy to another one, they are required to call the Controller.setStrategy function. This function is responsible for first withdrawing CRV and JPEG tokens from strategy contract into vault, then...

[WP-H9] _swapUniswapV2 may use an improper path which can cause a loss of the majority of the rewardTokens

Lines of code Vulnerability details function harvestuint256 minOutCurve external onlyRoleSTRATEGISTROLE convexConfig.baseRewardPool.getRewardaddressthis, true; //Prevent Stack too deep errors DexConfig memory dex = dexConfig; IERC20 memory rewardTokens = strategyConfig.rewardTokens; IERC20 weth =...