Orderers Can Unauthorized Transfer User's Share In Single Step

Lines of code Vulnerability details Impact The transferFrom function of vToken.sol can be done without any user permissions or strict security checks, requires only the caller must has ORDERERROLE as the access control, exposing it to the centralize risk if an orderer is compromised or act...

Missing Validations for the return values of Chainlink Price feeds

Lines of code Vulnerability details Impact You check only the answerThe price after calling the chainlink Chainlink Price feeds in the following lines. In addition, you need to check whether the data is really updated. Proof of Concept Tools Used code review Recommended Mitigation Steps Please ad...

UniswapV2PriceOracle.sol currentCumulativePrices() will revert when priceCumulative addition overflow

Lines of code Vulnerability details uint price0Cumulative, uint price1Cumulative, uint32 blockTimestamp = addresspair.currentCumulativePrices; Because the Solidity version used by the current implementation of UniswapV2OracleLibrary.sol is =0.8.7, and there are some breaking changes in Solidity...

Index Minting and Redemption Can Be Frontrun

Lines of code Vulnerability details Impact According to the provided source code, the user must transfer the underlying asset to the contract prior to calling mint or the index token before to calling burn. If these two actions are performed on the difference block, it introduces the risk that...

Index mint and burn calls can be front run

Lines of code Vulnerability details Impact Both in the mint and burn cases all the user supplied / due to a user assets can be stolen by an attacker, who detects correspondingly asset transfer calls / Index token transfer call and front runs Index contract's mint / burn call with own address as a...

ORACLE Data is not properly validated in ChainlinkPriceOracle.sol

Lines of code Vulnerability details Impact Price can be stale which can lead to wrong assetPerBaseInUQ return value Proof of Concept Oracle data feed is insufficiently validated. There is no check for stale price and round completeness. Tools Used Manual review, similar issue was found in yield...

Chainlink oracle data can be stale

Lines of code Vulnerability details Impact Oracle data can be stale which can lead to wrong calculations for balancing indexes. Proof of Concept When lastRoundData is called only price is pulled from the provided data. RoundId should be checked to ensure the data is updated. Recommended Mitigatio...

Attacker can cause minter to lose his funds by calling sync() between deposit and mint()

Lines of code Vulnerability details Impact All funds will be lost Proof of Concept To mint new index tokens underlying tokens need to be deposited in Vtoken.sol and then call the mint function. The mint function does balance-previousBalance to calculate tokens that were deposited. The previous...

Chainlink's latestRoundData might return stale or incorrect results

Lines of code Vulnerability details , int basePrice, , , = baseAggregator.latestRoundData; On ChainlinkPriceOracle.sol, we are using latestRoundData, but there is no check if the return value indicates stale data. This could lead to stale prices according to the Chainlink documentation:...

basePrice and quotePrice values are insufficiently validated

Lines of code Vulnerability details Impact The oracle data feed basePrice and quotePrice in refreshedAssetPerBaseInUQ of ChainlinkPriceOracle.sol will be stale which results in wrong basePrice value and quotePrice value. Proof of Concept In refreshedAssetPerBaseInUQ function, it calls...

Index managers can rug user funds

Lines of code Vulnerability details Impact The ORDERERROLE role has the ability to arbitrarily transfer user funds, and this role is shared between both the orderer and people who can rebalance the index. Even if the owner is benevolent the fact that there is a rug vector available may negatively...

Price can be stale leading to wrong return value in refreshedAssetPerBaseInUQ in ChainlinkPriceOracle.sol

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. Stale price from data feed can lead to incorrect value of assetPerBaseInUQ Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that...

UniswapV2PriceOracle#refreshedAssetPerBaseInUQ() will revert when pair cumulative prices underflow

Lines of code Vulnerability details function refreshedAssetPerBaseInUQaddress asset external override returns uint uint price0Cumulative, uint price1Cumulative, uint32 blockTimestamp = addresspair.currentCumulativePrices; uint32 timeElapsed = blockTimestamp - blockTimestampLast; if timeElapsed =...

ERC20Gauges: The _incrementGaugeWeight function does not check the gauge parameter enough, so the user may lose rewards.

Lines of code Vulnerability details Impact The incrementGaugeWeight function is used to increase the user's weight on the gauge. However, in the incrementGaugeWeight function, it is only checked that the gauge parameter is not in deprecatedGauges, but not checked that the gauge parameter is in...

deposit()ing when there is no discount results in zero xCitadel bought

Lines of code Vulnerability details The amount of citadel bought when there is no discount is always zero. If the user doesn't specify, or specifies zero as the minCitadelOut, then the user will get no xCitadel and will still have to pay the full price. Proof of Concept If funding.discount is equ...

first depositor in StakedCitadel can drain other depositors

Lines of code Vulnerability details in deposit, when the ratio totalSupply / balance is very high, the amount of the minted shares can round down to zero. Proof of Concept Let's say that the token is USDC. Alice is the first one to deposit in StakedCitadel. she deposits 1 basic unit of USDC 10-6...

DoS in StakedCitadel._withdraw() due to missing StakedCitadelVester.setupVesting() function

Lines of code Vulnerability details Contract StakedCitadelVester inherits from interface IVesting in fact it does not as it is missing the necessary is IVesting statement, but it's assumed to inherit from IVesting but wrongly implements the interface. The contract is expected to implement the...

TokenInLimit can be set higher than what the contract has to sell

Lines of code Vulnerability details Impact If totalTokenOutBought tokenOut.balanceOfaddressthis all tokens will be unclaimable and all funds would be lost. Proof of Concept TokenInLimit can be set arbitrarily high even if the contract doesn't have enough tokens to sell. Then users can buy more...

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...

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...

StakedCitadel depositors can be attacked by the first depositor with depressing of vault token denomination

Lines of code Vulnerability details Impact An attacker can become the first depositor for a recently created StakedCitadel contract, providing a tiny amount of Citadel tokens by calling deposit1 raw values here, 1 is 1 wei, 1e18 is 1 Citadel as it has 18 decimals. Then the attacker can directly...

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...

Chainlink's latestRoundData missing security checks

Lines of code Vulnerability details Impact Protocol uses Chainlink as one of the oracles that provides prices for the assets. Chainlink's latestRoundData is used but the implementation is missing important security checks that can result in stale and incorrect prices being returned. Proof of...

IndexLogic: An attacker can burn other users' tokens to transfer assets to himself

Lines of code Vulnerability details Impact In the burn function of the IndexLogic contract, the user needs to transfer the tokens to the contract first, and then call the burn function to withdraw the assets. The attacker can monitor the number of tokens in the contract. When it is not 0, the...

Funding: citadelPriceInAsset is only lazily updated

Lines of code Vulnerability details Impact When using citadelPriceInAsset in the getAmountOut function, since citadelPriceInAsset is updated lazily, this will make users use stale citadelPriceInAsset , causing users to suffer asset loss. Proof of Concept Tools Used None Recommended Mitigation Ste...

In Funding.sol#getAmountOut(), citadelAmount_ doesnot account for case of discount

Lines of code Vulnerability details Impact Function getAmountOutassetAmountIn fails to handle the case for funding.discount == 0 Proof of Concept function getAmountOutuint256 assetAmountIn public view returns uint256 citadelAmount uint256 citadelAmountWithoutDiscount = assetAmountIn...

Is Possible Attacker Block setStrategy() When Already Existing Strategy

Lines of code Vulnerability details Impact /// NOTE: Migrate funds if settings strategy when already existing one if strategy != address0 require IStrategystrategy.balanceOf == 0, "Please withdrawToVault before changing strat" ; When setStrategy was called, it require no fund in existing Strategy...

If the funding discount is set to 0 in Funding.sol, getAmountOut() will return always 0, no matter the amount of asset in.

Lines of code Vulnerability details Impact If a user doesn't set a minimum set to 0 of citadel expected in function deposit, can receive 0 tokens in exchange, no matter the amount of asset he's depositing. Proof of Concept The funding contract is used to trade citadel token with another asset. A...

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...

Removing Provider could lead to no provider at all

Lines of code Vulnerability details Impact When removing a provider you don't verify that there is at least one provider left. If no provider is left, then no price data can be reported. Proof of Concept 1 Tools Used hardhat test Recommended Mitigation Steps Add a checks on the length of the...

Economic calculation may not be precise enough

Lines of code Vulnerability details In Funding.sol, the assetDecimalsNormalizationValue is set to 10 asset.decimals, later it is used to determine home many token units there are per one asset token in human representation. Firstly, this may be highly dangerous as .decimals return value isn't sai...

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...

the governance can mint citadel tokens for themselves

Lines of code Vulnerability details the governance can call mint in citadel token and mint for themselves as much as they want and sell, which will cause the token price to drop to zero. --- The text was updated successfully, but these errors were encountered: All reactions...

Linear vesting logic is incorrect for subsequent withdrawals

Lines of code Vulnerability details Impact After attempting a withdrawal, StakedCitadelVester.sol vest is called, creating a 21 day vesting schedule for the user to claim their withdrawed amount. This logic works perfectly for the first withdrawal, but will be incorrect for every subsequent...

New vest reset unlockBegin of existing vest without removing vested amount

Lines of code Vulnerability details Impact When vest is called by xCTDL vault, the previous amount will re-lock according to the new vesting timeline. While this is as described in L127, claimableBalance might revert due to underflow if vestingrecipient.claimedAmounts 0 because the user will need...

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...

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...

getPricePerFullShare() can be much lower than expected

Lines of code Vulnerability details Impact Since balance does not include the amount 95% that's been transferred to the strategy, the getPricePerFullShare may only give 5% of the intended price. getPricePerFullShare is used in other contracts, such as Funding.sol and may also be used in other...

admin can rug

Lines of code Vulnerability details the mint function in CitadelToken requires the role CITADELMINTERROLE. this role is managed by the governance: setRoleAdminCITADELMINTERROLE, CONTRACTGOVERNANCEROLE; therefore the admin can mint to himself an unlimited amount. --- The text was updated...

centralization risk

Lines of code Vulnerability details Impact Can lead to unlimited minting of tokens Proof of Concept If any of the provided roles / actors get malicious, then unlimited number for tokens either for mint or redeem, can lead to loss for the protocol. It should be onlyadmin based or either should be...

The name of the function to setup a vesting in the interface IVesting.sol doesn’t match with the name of the function to setup a vesting in StakeCitadelVester.sol.

Lines of code Vulnerability details Impact Users will not be able to withdraw their funds . Proof of Concept When a user wants to withdraw his tokens from StakedCitadel.sol, vesting is supposed to be set and tokens are sent to the vesting contract where they are vested linearly for 21 days. This ...

Duplicate asset can be added

Lines of code Vulnerability details Impact Initialize function can be called multiple times with same asset. Calling with same asset will make duplicate entries in assets list. Any function reading assets will get impacted and would retrieve duplicate asset Proof of Concept 1. Observe that...

Staked Citadel function balance and reportHarvest

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

DoS at CitadelMinter.sol

Lines of code Vulnerability details Impact At CitadelMinter.sol, Funding Pool Weight can't be set at the beginning since totalFundingPoolWeight value is not assigned and Zero meanwhile being cached to newTotalWeight. Hence the substraction will not perform as it will yield to a negative value whi...

loss of funds if there is no discount

Lines of code Vulnerability details citadelAmount is initialized to 0 by default, and then if the discount is greater than zero, it is set to the amount out. But if the discount is 0, it is not set to the amount out and citadelAmount stays 0 and then divided by assetDecimalsNormalizationValue, bu...

setStrategy() Can Happen Without Timelock

Lines of code Vulnerability details Impact /// @dev This is a rug vector, pay extremely close attention to the next strategy being set. /// Changing the strategy should happen only via timelock. function setStrategyaddress strategy external whenNotPaused onlyGovernance; requirestrategy != address...

_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...

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...

Reliance on ERC20Upgradable.decimals() will always return 18 despite actual token decimals

Lines of code Vulnerability details Impact A call to ERC20UpgradeabletokenIn.decimals is used in the getAmountOut function of KnightingRound.sol to determine how much citadel to provide to the user for the given amount of tokenIn. The issue with using ERC20Upgradeable.decimals is that it always...