Looping over dynamic array might result in DoS through the block gas limit

Handle Ruhum Vulnerability details Impact Because of the block gas limit, looping over a dynamic array that grows over time might result in a DoS at some point. Both the PoolTemplate and the IndexTemplate have such dynamic arrays. Both don't have any functionality to decrease the size. Meaning, i...

no check that DEFAULT_ADMIN_ROLE is not the contract itself in L1Escrow.sol

Handle jayjonah8 Vulnerability details Impact In L1Escrow.sol the constructor sets up the DEFAULTADMINROLE without requiring that the msgSender is not the L1Escrow contract itself. Proof of Concept constructor setupRoleDEFAULTADMINROLE, msgSender; Tools Used Manual code review Recommended...

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

Malicious Market Creators Can Steal Tokens From Unsuspecting Approved Reference Accounts

Handle leastwood Vulnerability details Impact The current method of market creation involves calling Factory.createMarket with a list of approved conditions and references accounts. If a registered template address has templatesaddresstemplate.isOpen == true, then any user is able to call...

requestWithdraw without obligation to withdraw allow underwriter to avoid payout

Handle gzeon Vulnerability details Impact To prevent withdrawal front-running, a lockup period is set between withdrawal request and withdrawal. However, there are no obligation to withdraw after the lockup period and the capital will keep earning premium during lockup. A strategy for underwriter...

Accounting for non-standard ERC20 fees

Handle Dravee Vulnerability details Impact Bad accounting in case of non-standard ERC20 tokens Proof of Concept Vault.sol 105: IERC20token.safeTransferFromfrom, addressthis, amount; 106: 107: balance += amount; Vault.sol 136: IERC20token.safeTransferFromfrom, addressthis, amount; 137: balance +=...

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

[WP-H39] PoolTemplate.sol#resume() Wrong implementation of resume() will compensate overmuch redeem amount from index pools

Handle WatchPug Vulnerability details Root Cause Wrong arithmetic. uint256 deductionFromIndex = debt totalCredit MAGICSCALE1E6 / totalLiquidity; uint256 actualDeduction; for uint256 i = 0; i 0 uint256 shareOfIndex = credit MAGICSCALE1E6 / totalCredit; uint256 redeemAmount = divCeil...

approve() function does not require that _token and _spender args are unique

Handle jayjonah8 Vulnerability details Impact In L1Escrow.sol the approve function takes in a token to approve the spender to be able to spend but does not check that the token and spender are different addresses which can leave the protocol open to be expo;ited. Proof of Concept function approve...

unlock function is callable by anyone

Handle Fitraldys Vulnerability details Impact In the the unlock function is callable by anyone, and there is no check if the caller is the correct insured address or not, this can be dangerous because it relies on parameters.getGracemsg.sender, which if the msg.sender didnt have any grace, it wil...

[WP-H30] A malicious/compromised Registry or Factory admin can drain all the funds from the Vault contracts

Handle WatchPug Vulnerability details modifier onlyMarket require IRegistryregistry.isListedmsg.sender, "ERRORONLYMARKET" ; ; function borrowValueuint256 amount, address to external onlyMarket override debtsmsg.sender += amount; totalDebt += amount; IERC20token.safeTransferto, amount; The current...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Lack of slippage checks during swap

Handle ye0lde Vulnerability details Impact Since the code does not use a minimum return value for swaps it is susceptible to sandwich attacks. More information here: Proof of Concept Here the code Calls Curve to convert the existing underlying balance into UST: / Calls Curve to convert the existi...