applyCover() Does Not Enforce Index Market Lock

Handle leastwood Vulnerability details Impact The applyCover function is called by the insurance pool owner and intends to store data related to an insurance incident. Upon function execution, applyCover iterates over all available index markets and calls lock, denying all deposits and withdrawal...

backdoor in withdrawRedundant

Handle cmichel Vulnerability details The Vault.withdrawRedundant has wrong logic that allows the admins to steal the underlying vault token. function withdrawRedundantaddress token, address to external override onlyOwner if token == addresstoken && balance 0 // @audit they can rug users. let's sa...

Initial pool deposit can be stolen

Handle cmichel Vulnerability details Note that the PoolTemplate.initialize function, called when creating a market with Factory.createMarket, calls a vault function to transfer an initial deposit amount conditions1 from the initial depositor references4: // PoolTemplate function initialize string...

Vault.withdrawRedundant() allows the owner to accidentally take out the vault's whole balance

Handle Ruhum Vulnerability details Impact The Vault.withdrawRedundant allows the owner to withdraw funds that are not accounted for. The function has a check that is supposed to stop the owner from withdrawing funds of the vault's underlying token that the vault "knows" about. But, there's an edg...

Out of gas.

Handle Jujic Vulnerability details Impact There is no upper limit on allMarkets, it increments each time when a new market is added. Eventually, as the count of markets increases, gas cost of smart contract calls will raise until reaching an "Out of Gas" error or a "Block Gas Limit" in the worst...

Index compensate is 0 when totalLiquidity() is enough to cover the whole amount

Handle pauliax Vulnerability details Impact In IndexTemplate, function compensate, When amount value, and = totalLiquidity, the value of compensated is not set, so it gets a default value of 0: if value = amount ... compensated = amount; else ... if totalLiquidity amount ... compensated = value +...

Looping from a long list of storage can impact other people paying more gas than it used to

Handle Fitraldys Vulnerability details Impact In the it will loop through an entire indexList array this doesnt immedietely impact other user, however when there is many user call allocateCredit this function will add another indexlist if the user didnt exist in the first place. Lets say the time...

No check that DEFAULT_ADMIN_ROLE is not the LivepeerToken contract itself

Handle jayjonah8 Vulnerability details Impact In LivepeerToken.sol the constructor sets the DEFAULTADMINROLE but does not ensure that the msgSender is not the contract itself. This is an important check to make in order to avoid costly mistakes during deployment. Proof of Concept LivepeerToken.so...

[WP-H33] IndexTemplate.sol Wrong implementation allows lp of the index pool to resume a locked PayingOut pool and escape the responsibility for the compensation

Handle WatchPug Vulnerability details Based on the context, the system intends to lock all the lps during PayingOut period. However, the current implementation allows anyone, including LPs to call resume and unlock the index pool. It allows a malicious LP to escape the responsibility for the...

Unbounded iteration over all pools

Handle Dravee Vulnerability details Impact The transactions could fail if the array get too big and the transaction would consume more gas than the block limit. This will then result in a denial of service for the desired functionality and break core functionality. Proof of Concept Tools Used VS...

Unbounded iteration over all indexes

Handle Dravee Vulnerability details Impact The transactions could fail if the array get too big and the transaction would consume more gas than the block limit. This will then result in a denial of service for the desired functionality and break core functionality. Proof of Concept Tools Used VS...

[WP-M1] withdraw() transactions can often fail

Handle WatchPug Vulnerability details function withdraw address to, uint256 memory ids, bool force internal uint256 localTotalShares = totalShares; uint256 localTotalPrincipal = totalUnderlyingMinusSponsored; uint256 amount; for uint8 i = 0; i ids.length; i++ amount += withdrawDeposit idsi,...

Typo in PoolTemplate unlock function results in user being able to unlock multiple times

Handle loop Vulnerability details The function unlock in PoolTemplate has a typo where it compares insurancesid.status to false rather than setting it to false. If the conditions are met to unlock the funds for an id, the user should be able to call the unlock function once for that id as...

[WP-H12] forceUnsponsor() may open a window for attackers to manipulate the _totalShares and freeze users' funds at a certain deposit amount

Handle WatchPug Vulnerability details if force && sponsorAmount totalUnderlying sponsorToTransfer = totalUnderlying; else if !force require sponsorToTransfer totalUnderlying, the contract will transfer totalUnderlying to sponsorToTransfer, even if there are other depositors and totalShares 0. Aft...

Medium: Consider alternative price feed + ensure _minLockPeriod > 0 to prevent flash loan attacks

Handle hickuphh3 Vulnerability details Impact It is critical to ensure that minLockPeriod 0 because it is immutable and cannot be changed once set. A zero minLockPeriod will allow for flash loan attacks to occur. Vaults utilising the nonUST strategy are especially susceptible to this attack vecto...

Vault: Reduce reliance on manual rebalances from strategy to vault

Handle hickuphh3 Vulnerability details Impact Withdrawals are processed solely with funds that are held by the vault. Should there be insufficient liquidity Eg. many withdrawals in a short time, users have to rely on a trusted party operator to move funds from the investment strategy to the vault...

wring distribution of debts

Handle danb Vulnerability details redeemAmount should be deductionFromIndex multiplied by shareOfIndex, not divided. this would lead to a wrong distribution of payments. --- The text was updated successfully, but these errors were encountered: All reactions...

[WP-M4] NonUSTStrategy.sol Lack of slippage control

Handle WatchPug Vulnerability details function swapUnderlyingToUst internal uint256 underlyingBalance = getUnderlyingBalance; if underlyingBalance 0 // slither-disable-next-line unused-return curvePool.exchangeunderlying underlyingI, ustI, underlyingBalance, 0 ; The current implementation of...

Depositors are at mercy of admins to get their funds back

Handle cmichel Vulnerability details When depositors want their funds back and there are not enough funds in the vault or the strategy, the aUST in the BaseStrategy needs to be redeemed. This redemption process is asynchronous due to the nature of EthAnchor and requires an admin to redeem the aUS...

Missing slippage/min-return check in NonUSTStrategy

Handle cmichel Vulnerability details The contracts are missing slippage checks which can lead to being vulnerable to sandwich attacks. A common attack in DeFi is the sandwich attack. Upon observing a trade of asset X for asset Y, an attacker frontruns the victim trade by also buying asset Y, lets...

Add a timelock to BaseStrategy:setPerfFeePct

Handle Dravee Vulnerability details Impact To give more trust to users: functions that set key/critical variables should be put behind a timelock. Proof of Concept Tools Used VS Code Recommended Mitigation Steps Add a timelock to setter functions of key/critical variables. --- The text was update...

Race condition to withdraw funds

Handle pedroais Vulnerability details Impact Users may have to wait more time than predicted to withdraw funds. Proof of Concept The vault's funds are distributed between the vault and the investment strategy but withdrawals come only from funds that are currently inside the vault. This means tha...

Vault withdraws should withdraw from strategy if necessary

Handle harleythedog Vulnerability details Impact When a user calls withdraw, the amount of underlying assets that they are owed is dependent on their number of shares, and the number of underlying assets in the vault + strategy. If x is the number of underlying tokens intended to be sent to the...

Missing slippage/min-return check in the curve Pool

Handle defsec Vulnerability details Impact Trades can happen at a bad price and lead to receiving fewer tokens than at a fair market price. The attacker's profit is the protocol's loss. Proof of Concept The NonUSTStrategy contract is missing slippage checks which can lead to being vulnerable to...

require check doesnt comply with an underlying token that implemented fee in the transfer and transferFrom function

Handle Tomio Vulnerability details Impact The implementation for the transferAndCheckUnderlying function can revert the transaction if the underlying token is implementing a fee on the transfer and transferFrom function, this can happen because when a token implementing a fee on transfer, it...

Slippage protection

Handle pauliax Vulnerability details Impact exchangeunderlying in functions swapUnderlyingToUst and swapUstToUnderlying lack slippage control, it uses a default value of 0 minimum received. A common attack in DeFi is the sandwich attack. Upon observing a trade of asset X for asset Y, an attacker...

Unsafe mint is a reentrancy door

Handle pedroais Vulnerability details Impact Dangerous external calls in the middle of various state changes could cause reentrancy issues since there is no reentrancy guard in any functions. Proof of Concept When users call the deposit or sponsor functions a deposit NFT is minted. The safeMint...

Re-entrancy in _createDeposit allows stealing tokens

Handle cmichel Vulnerability details The Vault.createDeposit function first caches the current total shares and underlying, and then iterates over all claims using a call to createClaim. Only afterwards, does it pull in the required total amount in the deposit. function depositDepositParams...

A Single Malicious Trusted Account Can Takeover Parent Contract

Handle leastwood Vulnerability details Impact The requiresTrust modifier is used on the strategy, vault and factory contracts to prevent unauthorised accounts from calling restricted functions. Once an account is considered trusted, they are allowed to add and remove accounts by calling...

Missing reentrant check can be abused by increasing total share, before transfering underlying token

Handle Fitraldys Vulnerability details Impact When interacting with the vault contract, the user can call the deposit function, however this can be used to reenter this function because the deposit function will eventually call depositors.mint and in the mint function inside depositors will call...

Vault: onDepositMinted() reentrancy causes users’ deposits to be erroneously accounted as yield

Handle hickuphh3 Vulnerability details Impact The deposit function does not have reentrancy protection. This allows reentrancy to occur through the implementation of a malicious claim’s beneficiary onDepositMinted function that will cause all users’ deposits to be erroneously interpreted as yield...

No slippage protection on _swapUnderlyingToUst can lead to lost funds

Handle harleythedog Vulnerability details Impact The function swapUnderlyingToUst exists to swap underlying tokens to Ust. The last argument to exchangeunderlying is mindy, which specifies the minimum number of Ust to be returned from the swap. Currently, this value is set to 0, so the function i...

Missing Re-entrancy Guard

Handle defsec Vulnerability details Impact The re-entrancy guard is missing on the Eth anchor interaction. The external router interaction can cause to the re-entrancy vulnerability. Proof of Concept 1. Navigate to the following contract. function finishDepositStableuint256 idx external...

[WP-H0] Late users will take more losses than expected when the underlying contract (EthAnchor) suffers investment losses

Handle WatchPug Vulnerability details Even though it's unlikely in practice, but in theory, the underlying contract EthAnchor may suffer investment losses and causing decreasing of the PPS of AUST token. There are codes that considered this situation in the codebase. eg. handling of depositShares...

Incompatibility With Rebasing/Deflationary/Inflationary tokens

Handle defsec Vulnerability details Impact The Strategy contracts do not appear to support rebasing/deflationary/inflationary tokens whose balance changes during transfers or over time. The necessary checks include at least verifying the amount of tokens transferred to contracts before and after...

[WP-H9] Centralization Risk: Funds can be frozen when critical key holders lose access to their keys

Handle WatchPug Vulnerability details The current implementation requires trusted key holders isTrustedmsg.sender to send transactions initRedeemStable to initialize withdrawals from EthAnchor before the users can withdraw funds from the contract. This introduces a high centralization risk, which...

Check _to is not empty

Handle pauliax Vulnerability details Impact functions claimYield, withdraw, and unsponsor should validate that to is not an empty 0x0 address to prevent accidental burns. Recommended Mitigation Steps Consider implementing the proposed validation: require to != address0 --- The text was updated...

[WP-N7] Vault.sol Tokens with fee on transfer are not supported

Handle WatchPug Vulnerability details There are ERC20 tokens that charge fee for every transfer or transferFrom. Vault.soltransferAndCheckUnderlying requires that the received amount is the same as the transfer amount, otherwise, it will revert at L587. function transferAndCheckUnderlyingaddress...

Well-Funded Sponsors Can Severely Inhibit The Availability Of The Protocol

Handle leastwood Vulnerability details Impact Any user is allowed to sponsor a vault by depositing tokens, thereby bootstrapping the vault's initial assets used in the strategy contract. A percentage of these assets are invested in the Anchor protocol by bridging UST from Ethereum to the Terra...

iteration over unbounded array

Handle danb Vulnerability details the functions applyCover and resume iterate over unbounded array. anyone can create an index and add it to the pool, therefore it is possible to add many indexes and prevent any call to applyCover and resume because of passing the gas limit. --- The text was...

[WP-H2] NonUSTStrategy.sol Improper handling of swap fees allows attacker to steal funds from other users

Handle WatchPug Vulnerability details NonUSTStrategy will swap the deposited non-UST assets into UST before depositing to EthAnchor. However, the swap fee is not attributed to the depositor correctly like many other yield farming vaults involving swaps ZapIn. An attacker can exploit it for the sw...

Owner can call applyCover multiple times in PoolTemplate.sol

Handle camden Vulnerability details Impact The owner could potentially extend the insurance period indefinitely in the applyCover function without ever allowing the market to resume. This is because there is no check in applyCover to ensure that the market is in a Trading state. This can also all...

Re-entrancy in _withdraw allows stealing tokens

Handle cmichel Vulnerability details The Vault.withdraw function first caches the current total shares and underlying, and then iterates over all deposit NFT ids using a call to withdrawDeposit. Only afterwards, does it pay out the accumulated withdrawn amount. function withdraw address to, uint2...

[WP-H10] A malicious early user/attacker can manipulate the pps to freeze users' funds at a certain deposit amount

Handle WatchPug Vulnerability details uint256 tokenId = depositors.mint msgSender, amount, claimerId, lockedUntil ; function safeMint address to, uint256 tokenId, bytes memory data internal virtual mintto, tokenId; require checkOnERC721Receivedaddress0, to, tokenId, data, "ERC721: transfer to non...

No slippage protection on _swapUstToUnderlying can lead to lost funds

Handle harleythedog Vulnerability details Impact The function swapUstToUnderlying exists to swap Ust to underlying tokens. The last argument to exchangeunderlying is mindy, which specifies the minimum number of underlying to be returned from the swap. Currently, this value is set to 0, so the...

Vault: onDepositBurn() causes users to unfairly have their deposits allocated as yield

Handle hickuphh3 Vulnerability details Impact The withdraw and forceWithdraw functions do not have reentrancy protection. This allows reentrancy to occur through the implementation of a malicious claim’s beneficiary onDepositBurn function that will cause the incorrect amount of shares to be minte...

Signature replay

Handle 0x1f8b Vulnerability details Impact Signature replay in PoolTemplate. Proof of Concept The redeem method of PoolTemplate verifies the data stored in incident, and the verification logic of this process is performed as following: require MerkleProof.verify merkleProof, targets, keccak256...

[WP-M8] totalUnderlyingMinusSponsored() may revert on underflow and malfunction the contract

Handle WatchPug Vulnerability details function totalUnderlyingMinusSponsored public view returns uint256 // TODO no invested amount yet return totalUnderlying - totalSponsored; As a function that many other functions depended on, totalUnderlyingMinusSponsored can revert on underflow when...

NonUSTStrategy invested assets can be manipulated

Handle cmichel Vulnerability details The NonUSTStrategy.investedAssets computes the value of the strategy by checking the output of a swap from the UST assets to underlying. This essentially uses Curve's UST/underlying spot price which can be manipulated. function investedAssets external view...

Both _mint and _safeMint are used to mint NFTs

Handle palina Vulnerability details Impact Both mint and safeMint functions are used to mint ERC721 NFTs in Claimers and Depositors, respectively. The usage of the mint is, however, discouraged by the used ERC721 implementation see PoC section, in favor of its safe counterpart. Proof of Concept...