Lack of access control allow attacker to mintFungible() and mintSynth() with other user's wallet balance

Handle WatchPug Vulnerability details function mintFungible IERC20 foreignAsset, uint256 nativeDeposit, uint256 foreignDeposit, address from, address to external override nonReentrant returns uint256 liquidity IERC20Extended lp = wrapper.tokensforeignAsset; require lp != IERC20ExtendedZEROADDRESS...

VaderPoolV2.mintFungible exposes users to unlimited slippage

Handle TomFrench Vulnerability details Impact Frontrunners can extract up to 100% of the value provided by LPs to VaderPoolV2. Proof of Concept Users can provide liquidity to VaderPoolV2 through the mintFungible function. This allows users to provide tokens in any ratio and the pool will calculat...

TWAPOracle.getRate does not scale the ratio

Handle cmichel Vulnerability details The TWAPOracle.getRate function simply performs an integer division to compute the rate. function getRate public view returns uint256 result uint256 tUSDInUSDV = consultUSDV; uint256 tUSDInVader = consultVADER; // @audit shouldn't this scale by 1e18 first?...

Potential economic attack that exploits IL coverage

Handle WatchPug Vulnerability details The current implementation of Vader protocol provides impermanent loss coverage calculated as below: function calculateLoss uint256 originalVader, uint256 originalAsset, uint256 releasedVader, uint256 releasedAsset public pure returns uint256 loss // // TODO:...

TWAPOracle might register with wrong token order

Handle cmichel Vulnerability details The TWAPOracle.registerPair function takes in a factory and token0, token1. The function accepts a factory argument which means any Uniswap-like factory can be used. When using the actual Uniswap factory's IUniswapV2Factoryfactory.getPairtoken0, token1 call, i...

SettToRenIbbtcZap.sol#setGuardian doesn't set the guardian, but governance

Handle 0x0x0x Vulnerability details Proof of Concept This function is intended to change the guardian, but the code changes the governance address as seen in the code: function setGuardianaddress guardian external onlyGovernance; governance = guardian; It should be replaced with the following cod...

IbbtcVaultZap.sol#setGuardian doesn't set the guardian, but governance

Handle 0x0x0x Vulnerability details Proof of Concept This function is intended to change the guardian, but the code changes the governance address as seen in the code: function setGuardianaddress guardian external onlyGovernance; governance = guardian; It should be replaced with the following cod...

Governor's veto protection can be exploited

Handle cmichel Vulnerability details The GovernorAlpha's council cannot veto proposals that perform a call to the contract itself. This can be exploited by malicious proposal creators by appending a new call at the end of their proposal that simply calls an innocent function like...

Lacking Validation Of Chainlink' Oracle Queries

Handle leastwood Vulnerability details Impact TwapOracle.consult is missing additional validations to ensure that the round is complete and has returned a valid/expected price. The consult improperly casts an int256 price to uint256 without first checking the value. As a result, the variable may...

BasePool.mint() Is Callable By Anyone

Handle leastwood Vulnerability details Impact The BasePool.mint function differs from its implementation in BasePoolV2.mint in which it lacks an onlyRouter modifier. This ensures that users cannot call this function directly as VaderRouter.addLiquidity performs some necessary input validation whi...

Covering impermanent loss allows profiting off asymmetric liquidity provision at expense of reserve holdings

Handle hyh Vulnerability details Impact Pool funds will be siphoned out over time as swaps and asymmetric LP provision are balancing each other economically, while with introduction of IL reimbursement a malicious user can profit immediately from out of balance pool with a swap and profit again...

No way to remove GasThrottle after deployment

Handle TomFrench Vulnerability details Impact Potential DOS on swaps Proof of Concept BasePool and BasePoolV2 make use of a validateGas modifier on swaps which checks that the user's gas price is below the value returned by FASTGASORACLE. Should FASTGASORACLE be compromised to always return zero...

XVader.sol : loss of user funds during 'leave' function

Handle shri4net Vulnerability details Impact Unintended Fee is charged to user while using 'leave' function in XVader.sol as part of the voting/governance. Proof of Concept Ref contracts/x-vader/XVader.sol, function leave Ref contracts/tokens/Vader.sol, function transfer Example, User locks 100...

mintSynth() and burnSynth() can be front run

Handle WatchPug Vulnerability details Given that mintSynth and burnSynth will issue and redeem assets based on the price of the pool reserves, and they will create price impact based on the volume being minted and burnt. However, the current implementation provides no parameter for slippage...

Unrestricted vestFor

Handle pauliax Vulnerability details Impact Anyone can call function vestFor and block any user with a tiny amount of Vader. This function has no auth checks so a malicious actor can front-run legit vestFor calls with insignificant amounts. This function locks the user for 365 days and does not...

Synth tokens can get over-minted

Handle WatchPug Vulnerability details Per the document: It also is capable of using liquidity units as collateral for synthetic assets, of which it will always have guaranteed redemption liquidity for. However, in the current implementation, Synth tokens are minted based on the calculation result...

Newly Registered Assets Skew Consultation Results

Handle leastwood Vulnerability details Impact The TwapOracle.consult function iterates over all token pairs which belong to either VADER or USDV and then calculates the price of the respective asset by using both UniswapV2 and Chainlink price data. This helps to further protect against price...

Unused slippage params

Handle pauliax Vulnerability details Impact Unused slippage params. function addLiquidity in VaderRouter both V1 and V2 do not use slippage parameters: uint256, // amountAMin = unused uint256, // amountBMin = unused making it susceptible to sandwich attacks / MEV. For a more detailed explanation,...

Unbounded loops

Handle pauliax Vulnerability details Impact There are several loops in the contract which can eventually grow so large as to make future operations of the contract cost too much gas to fit in a block. Specifically, in contract TwapOracle there is no upper boundary on how many pairs can be...

Attacker can claim more IL by manipulating pool price then removeLiquidity

Handle gzeon Vulnerability details Impact Vader reimburse user IL immediately when user withdraw from the pool VaderRouterV2.sol:L227, an attacker can therefore manipulate the pool balance causing a high IL, remove liquidity and restore the pool balance such that he will receive a larger IL...

VaderPoolV2 minting synths & fungibles can be frontrun

Handle cmichel Vulnerability details The VaderPoolV2 mintFungible and mintSynth functions perform an unsafe nativeAsset.safeTransferFromfrom, addressthis, nativeDeposit with a parameter-specified from address. Note that these functions are not called by the Router, they are directly called on the...

BasePool.swap() Is Callable By Anyone

Handle leastwood Vulnerability details Impact The BasePool.swap function differs from its implementation in BasePoolV2.swap in which it lacks an onlyRouter modifier. This ensures that users cannot call this function directly as VaderRouter.swap performs some necessary input validation which can b...

VaderRouter._swap performs wrong swap

Handle cmichel Vulnerability details The 3-path hop in VaderRouter.swap is supposed to first swap foreign assets to native assets, and then the received native assets to different foreign assets again. The pool.swapnativeAmountIn, foreignAmountIn accepts the foreign amount as the second argument...

Tokens with fee on transfer are not supported

Handle WatchPug Vulnerability details There are ERC20 tokens that charge fee for every transfer or transferFrom, E.g Vader token. In the current implementation, BasePoolV2.solmint assumes that the received amount is the same as the transfer amount, and uses it to calculate liquidity units. functi...

Unclear TwapOracle.consult algorithm

Handle cmichel Vulnerability details The TWAPOracle.consult function is unclear to the auditor. It seems to iterate through all registered pairs that share the token parameter USDV or VADER and then sums up the foreign token pair per token price. And divides this sum sumNative by the summed-up US...

No slippage protection in VaderPoolV2.mintSynth

Handle cmichel Vulnerability details The VaderPoolV2.mintSynth implicitly performs a "native - foreign" swap using VaderMath.calculateSwapnativeDeposit,reserveNative,reserveForeign, the resulting amount will be minted as synths instead of transferred out as foreign tokens. The calculateSwap...

Mixing different types of LP shares can lead to losses for Synth holders

Handle hyh Vulnerability details Impact Users that mint Synths do not get pool shares, so exiting of normal LP can lead to their losses as no funds can be left for retrieval. Proof of Concept 3 types of mint/burn: NFT, Fungible and Synths. Synths are most vilnerable as they do not have share: LP...

All user assets which are approved to VaderPoolV2 may be stolen

Handle TomFrench Vulnerability details Impact Total loss of funds which have been approved on VaderPoolV2 Proof of Concept VaderPoolV2 allows minting of fungible LP tokens with the mintFungible function Crucially this function allows a user supplied value for from which specifies where the...

Early user can break addLiquidity

Handle WatchPug Vulnerability details uint256 totalLiquidityUnits = totalSupply; if totalLiquidityUnits == 0 liquidity = nativeDeposit; // TODO: Contact ThorChain on proper approach In the current implementation, the first liquidity takes the nativeDeposit amount and uses it directly. However,...

Vests can be denied

Handle cmichel Vulnerability details The LinearVesting.vestFor function which is called by Converter reverts if there already exists a vest for the user: require vestuser.amount == 0, "LinearVesting::selfVest: Already a vester" ; There's an attack where a griefer frontruns the vestFor call and...

VaderPoolV2.rescue results in loss of funds rather than recoverability

Handle TomFrench Vulnerability details Impact Any unaccounted for tokens on VaderPoolV2 can be siphoned off by anyone Proof of Concept VaderPoolV2 has a rescue function which allows any unaccounted for tokens to be recovered. However there is no access control on this function which means than...

Incorrect Price Consultation Results

Handle leastwood Vulnerability details Impact The TwapOracle.consult function iterates over all token pairs which belong to either VADER or USDV and then calculates the price of the respective asset by using both UniswapV2 and Chainlink price data. This helps to further protect against price...

SHOULD CHECK RETURN DATA FROM CHAINLINK AGGREGATORS

Handle defsec Vulnerability details Impact The consult function in the contract TwapOracle.sol fetches the asset price from a Chainlink aggregator using the latestRoundData function. However, there are no checks on timeStamp, resulting in stale prices. The oracle wrapper calls out to a chainlink...

TWAP Oracle inflexible _updatePeriod

Handle elprofesor Vulnerability details Impact Update periods in TWAP oracles reflect risk of an asset. Updating more frequently accurately prices an asset but increases capabilities of manipulation which should be harder with more stable assets, whereas longer update periods prevent manipulation...

User may not receive the full amount of IL compensation

Handle jonah1005 Vulnerability details Impact The user would not get full IL compensation if there's not enough funds in the reserve. VaderReserve.solL76-L91 VaderReserve.solL85 uint256 actualAmount = minreserve, amount; While this is reasonable, users should be able to specify the minimum receiv...

Zap contract's redeem() function might transfer more tokens than the user should be able to receive

Handle Ruhum Vulnerability details Impact The redeem function transfers the whole balance of the specified token to the user. That can be more than the amount the user should receive. Because at the time of the call to redeem the Zap contract's balance of the passed token might not be 0. Proof of...

Missing duplicate veto check

Handle defsec Vulnerability details Impact On the GovernorAlpha contract, function veto has been added. Although the function behaviour is expected, duplicate veto process has not been checked on that function. Proof of Concept 1. Navigate to following contract line. function vetouint256...

Zap contract's redeem() function doesn't check which token the user wants to receive

Handle Ruhum Vulnerability details Impact In the redeem function, the user can pass a token address. That's the token they receive in return for the ibbtc they give back. Because of missing address checks the user can provide any possible ERC20 token here without the function reverting. Although...

The first lp provider can destroy the pool

Handle jonah1005 Vulnerability details Impact First lp provider received liquidity amount same as the nativeDeposit amount and decides the rate. If the first lp sets the pool's rate to an extreme value no one can deposit to the pool afterward. please refer to the proof of concept section A...

add liquidity is vulnerable to sandwich attack

Handle jonah1005 Vulnerability details add liquidity is vulnerable to MEV Impact addLiquidity in the VaderRouter and VaderRouterV2 contract does not check the minimum liquidity amount. This makes users' funds vulnerable to sandwich attacks. The team says a minimum amount is not required as the...

Missing hasStarted modifier, can lead to user vesting before the owner begin the vesting

Handle rfa Vulnerability details Impact In the claimConverted function, the user can vest their vader token for a certain amount of time, but hasStarted modifier is missing, this can lead to claimConverted function is callable by anyone, and the user can claim eventhough the vesting havent been...

Copy your own portfolio to keep earning royalties

Handle jayjonah8 Vulnerability details Impact In NestedFactory.sol going through the create function which leads to the sendFeesWithRoyalties = addShares function, Im not seeing any checks preventing someone from copying their own portfolio and receiving royalty shares for it and simply repeating...

LinearVesting does not calculate vested amount linearly

Handle xYrYuYx Vulnerability details Impact These calculations are incorrect for linear vesting. Proof of Concept i.e. if start amount is 10000, and duration is 100 seconds. After 50 seconds, user can claim 5000 which is 50% After another 10 seconds, user need to claim 1000 which is 10%, but...

FSDVesting.claimTribute() Breaks Contract

Handle leastwood Vulnerability details Impact FSDVesting.claimTribute attempts to claim staking rewards which are stored as tributes and generated through membership purchases by the FSD network. The FSDVesting.sol contract accrues a conviction score which generates a percentage claim over tribut...

Multicall.sol batch calls don't update msg.value

Handle jayjonah8 Vulnerability details Impact In NestedFactory.sol using Multicall.sol can be dangerous when it has a msg.value inside a loop since the msg.value doesn't update every iteration. This can lead to a user sending ETH one time and it being counted for every iteration. There is a...

Anyone Can Arbitrarily Call FSDVesting.updateVestedTokens()

Handle leastwood Vulnerability details Impact The updateVestedTokens function is intended to be called by the FSD.sol contract when updating a user's vested token amount. A check is performed to ensure that user == beneficiary, however, as user is a user controlled argument, it is possible to spo...

TributeAccrual.availableTribute() & TributeAccrual.availableGovernanceTribute() Distributes Tributes Unfairly

Handle leastwood Vulnerability details Impact Conviction scores are calculating by taking the user's balance and multiplying it by the time elapsed. This score is updated upon each token transfer, or alternatively by directly calling ERC20ConvictionScore.updateConvictionScore. The availableTribut...

Users Can DOS Vesting Distributions

Handle leastwood Vulnerability details Impact The Vesting.vest function is called by airdrop/investor distributions to lock 70% of their token allocations for a period of one year. Vestings are defined on a linear schedule and can be claimed as often as the user likes. However, the claimableAmoun...

Users Can Frontrun revoke() By Calling vest()

Handle leastwood Vulnerability details Impact The onlyOwner role typically calls revoke if a member leaves the BootFinance team, resulting in vested tokens being transferred to the multisig account. Each vesting account has a revocable state variable that is set to either true or false. As any us...

Inconsistent balance when supplying transfer-on-fee or deflationary tokens

Handle Reigada Vulnerability details Impact In the contract StakingRewards, the stake function assume that the amount of stakingToken is transferred to the smart contract after calling the safeTransferFrom function and thus it updates the balances mapping. However, this may not be true if the...