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

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

modifier noContract() needs to be changed

Lines of code Vulnerability details Impact noContract NatSpec description is "Modifier that ensures that non-whitelisted contracts can't interact with the LP farm". It is already stated that "some contracts will be able to bypass this check" however the impact is miscalculated and necessary gas t...

Setting new controller can break YVaultLPFarming

Lines of code Vulnerability details Impact The accruals in yVaultLPFarming will fail if currentBalance previousBalance in computeUpdate. currentBalance = vault.balanceOfJPEG + jpeg.balanceOfaddressthis; uint256 newRewards = currentBalance - previousBalance; No funds can be withdrawn anymore as th...

FungibleAssetVaultForDAO: Use latestRoundData + validate data freshness

Lines of code Vulnerability details Details & Impact The deprecated latestAnswer API is being used, which may at any time fail to work if Chainlink ends support for it. In addition, the data freshness should be checked. The oracle could, for example, not have been updated in a while, causing...

Division before Multiplication May Result In No Interest Being Accrued

Lines of code Vulnerability details Impact There is a division before multiplication bug in NFTVault.calculateAdditionalInterest which may result in no interesting being accrued and will have significant rounding issues for tokens with small decimal places. This issue occurs since an intermediate...

setDebtInterestApr should accrue debt first

Lines of code Vulnerability details Impact The setDebtInterestApr changes the debt interest rate without first accruing the debt. This means that the new debt interest rate is applied retroactively to the unaccrued period on next accrue call. It should never be applied retroactively to a previous...

Reentrancy issue in yVault.deposit

Lines of code Vulnerability details Impact In deposit, the balance is cached and then a token.transferFrom is triggered which can lead to exploits if the token is a token that gives control to the sender, like ERC777 tokens. POC Initial state: balance = 1000, shares supply = 1000. Depositing 1000...

Controller: Strategy migration will fail

Lines of code Vulnerability details Details The controller calls the withdraw method to withdraw JPEGs from the contract, but the strategy might blacklist the JPEG asset, which is what the PUSDConvex strategy has done. The migration would therefore revert. Proof of Concept Insert this test into...

Usage of deprecated Chainlink's latestAnswer function

Lines of code Vulnerability details Impact Contract NFTVault.sol uses Chainlink's latestAnswer as an oracle for prices of multiple assets. This function will return the last value, but it is not possible to check if the provided data is fresh. In addition latestAnswer has been marked as deprecate...

Can steal yVault deposits due to bad initial shares calculation

Lines of code Vulnerability details Impact The yVault.deposit function mints initial shares equal to the deposited amount. The deposit / withdraw functions also use the balance, which includes the contract balance token.balanceOfaddressthis, to compute the shares. It's possible to increase the...

Locked amount incorrectly updated in JPEGLock.sol

Lines of code Vulnerability details Impact The lockAmount gets overwritten each time the lockFor function is called in JPEGLock.sol. If the function is called multiple times, the parameter will be updated wrongly. Proof of Concept The lockAmount parameter gets assigned with the amount locked each...

Wrong lockFor can lost tokens

Lines of code Vulnerability details Impact If the owner incorrectly uses the lockFor method and calls it twice with the same nftIndex, it will overwrite the record and the deposited jpeg can never be retrieved. Proof of Concept Owner call lockForAlice,0,100; Owner call lockForBob,0,100; 100 token...

Usage of deprecated Chainlink's latestAnswer function

Lines of code Vulnerability details Impact Contracts NFTVault.sol and FungibleAssetVaultForDAO use Chainlink's latestAnswer as an oracle for prices of multiple assets. This function will return the last value, but it is not possible to check if the provided data is fresh. In addition latestAnswer...

yVaultLPFarming: No guarantee JPEG currentBalance > previousBalance

Lines of code Vulnerability details Details & Impact yVault users participating in the farm have to trust that: vault.balanceOfJPEG returns the correct claimable JPEG amount by its strategy / strategies the strategy / strategies will send all claimable JPEG to the farm Should either of these...

For any (sufficiently regular) smart contract, there exists a smart contract that bypasses noContract modifier and still has the same functionality

Lines of code Vulnerability details Impact modifier noContractaddress account is supposed to fill the following role according to the docstring: "Modifier that ensures that non-whitelisted contracts can't interact with the farm. Prevents non-whitelisted 3rd party contracts e.g. autocompounders fr...

Use of deprecated API for chainlink aggregator

Lines of code Vulnerability details Impact Price returned by oracle could be stale Proof of Concept The use of aggregator.latestAnswer is deprecated and could lead to stale data if the data wasn't recently updated. This is especially important for checking NFT prices since prices can change quick...

Tokens will be forever burned if contract call never success. Moreover, it may not retriable after OLD_KEY_RETENTION + 1 epochs has passed.

Lines of code Vulnerability details Impact Tokens will be forever burned if contract call never success. Moreover, it may not retriable after OLDKEYRETENTION + 1 epochs has passed. Just setting commandExecuted flag to false is not sufficient. As contract call never success, it will never got...

Low level call returns true if the address doesn't exist

Lines of code Vulnerability details Impact As written in the solidity documentation, the low-level functions call, delegatecall and staticcall return true as their first return value if the account called is non-existent, as part of the design of the EVM. Account existence must be checked prior t...

User's funds can get lost when transferring to other chain

Lines of code Vulnerability details Impact When transferring tokens to other chain, the tokens in the source chain are burned - if they are external they will be transferred to the AxelarGateway, otherwise they will be burned. In the target chain the same amount of tokens will be minted for the...

Unsupported fee-on-transfer tokens

Lines of code Vulnerability details Impact When tokenAddress is fee-on-transfer tokens, in the burnTokenFrom function, the actual amount of tokens received by the contract will be less than the amount. Proof of Concept Tools Used None Recommended Mitigation Steps Consider getting the received...

Cross-chain smart contract calls can revert but source chain tokens remain burnt and are not refunded

Lines of code Vulnerability details Impact Smart contract calls often revert. In such cases any ether sent along with the transaction is returned and sometimes the remaining gas depending on whether an assert caused the reversion or not. For contracts involving ERC20 tokens it is also expected...

_execute can potentially reorder a batch of commands while executing, breaking any assumptions on command orders.

Lines of code Vulnerability details Impact Since this is important, we quote it again instead of referring to our other bug report on a different, yet related bug. The context within which a command is executed is extremely important. AxelarGatewayMultisig.execute takes a signed batch of commands...

User Who Lock Up JPEG More Than Once Will Permanantly Lock Their Previous Deposited JPEG into Contract

Lines of code Vulnerability details Impact 1. Alice Request DAO to Call setNFTTypeValueETH and DAO Approved it 2. Alice Call finalizePendingNFTValueETH and External Function jpegLocker.lockFor was Called. 3. In JPEGLock.sol, Alice Transfer lockAmount = 1000 of JPEGS into the Contract and info was...

UniV3LpVault flash loans can become unavailable if a user didn't utilized the full limit

Lines of code Vulnerability details Impact If a user didn't utilized the whole amount on the first call, the flashFocusCall can fail for second and subsequent runs when params.asset is a token that use approval race protection. Placing severity to medium as flashFocusCall reverts in this case, an...

Must approve 0 first

Lines of code Vulnerability details Impact Some tokens like USDT do not work when changing the allowance from an existing non-zero allowance value.They must first be approved by zero and then the actual allowance must be approved. Proof of Concept Tools Used None Recommended Mitigation Steps Use...

Not calling approve(0) before setting a new approval causes the call to revert when used with Tether (USDT)

Lines of code Vulnerability details Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether USDT's approve function will revert if the current approval is not zero, to protect against front-running changes of...

Undercollateralized loans possible

Lines of code Vulnerability details Impact The setPoolCollateralFactors function does not check that the collateral factor is 100%. It's possible that it's set to 200% and then borrows more than the collateral is worth, stealing from the pool. Recommended Mitigation Steps Disable the possibility ...

Improper Access Control

Lines of code Vulnerability details Impact In the referenced code this line, requiremsg.sender != admin, "caller not admin"; is meant to prevent non-admins from calling the function however it instead prevents admins from calling the function and allows anyone else to. This could lead to defacing...

Dysfunctional CToken._acceptAdmin due to lack of function to assign pendingAdmin

Lines of code Vulnerability details Impact The implementation of CToken in Duality introduced an acceptAdmin function, which presumably should allow changing the admin. However, there does not exist a pairing proposePendingAdmin function that can propose a new pendingAdmin, thus pendingAdmin will...

Comptroller._setUniV3LpVault will always cause in-use uniswapV3 positions to become stuck in UniV3LpVault

Lines of code Vulnerability details Impact Comptroller.setUniV3LpVault allows the admin of Comptroller to change the accompanying UniV3LpVault. However since actions including collateral calculation, uniswapV3 position withdrawal, uniswapV3 collateral liquidation all require Comptroller and...

Transferring admins does not work for CToken

Lines of code Vulnerability details Impact The CToken implements an acceptAdmin function that sets the new admin to the pendingAdmin. But CToken does not implement a setPendingAdmin function to set the pendingAdmin in the first place. Therefore, the acceptAdmin function is useless and CTokens can...

Arbitrary contract call within UniV3LpVault._swap with controllable swapPath

Lines of code Vulnerability details Impact UniV3LpVault.swap utilizes swapRouter.exactInput to perform swaps between two tokens. During swaps, transfer function of each token along the path will be called to propagate the assets. Since anyone can create a uniswap pair of arbitrary assets, it is...

Existing user’s locked JPEG could be overwritten by new user, causing permanent loss of JPEG funds

Lines of code Vulnerability details Details & Impact A user’s JPEG lock schedule can be overwritten by another user’s if he the other user submits and finalizes a proposal to change the same NFT index’s value. The existing user will be unable to withdraw his locked JPEGs, resulting in permanent...

yVault: First depositor can break minting of shares

Lines of code Vulnerability details Details The attack vector and impact is the same as TOB-YEARN-003, where users may not receive shares in exchange for their deposits if the total asset amount has been manipulated through a large “donation”. Proof of Concept Attacker deposits 1 wei to mint 1...

Wrong authorization for CToken._setNameAndSymbol

Lines of code Vulnerability details Impact Anyone can change a CToken's name and symbol because of a wrong admin check: This will not only be very confusing for CToken holders but also allows many phishing attacks that will lead to loss of funds. For example, a victim holding both cWBTC and cUSDC...

Take other lender loanTicket without improving any of the loan conditions

Lines of code Vulnerability details Impact Take other lender loanTicket without improving any of the loan conditions Proof of Concept The bug is in NftLoanFacilliator.sol in the function lend link1. && previousInterestRate - previousInterestRate requiredImprovementRate / SCALAR = interestRate, fo...

mintBorrowTicketTo can be a contract with no onERC721Received method, which may cause the BorrowTicket NFT to be frozen and put users' funds at risk

Lines of code Vulnerability details IERC721MintableborrowTicketContract.mintmintBorrowTicketTo, id; function mintaddress to, uint256 tokenId external override loanFacilitatorOnly mintto, tokenId; If mintBorrowTicketTo is a contract that does not implement the onERC721Received method, in the curre...

Lender can stop others from offering higher rates.

Lines of code NFTLoandFacilitator.lend; L205 Vulnerability details Impact A lender using an upgradeable smart contract could stop other lenders from buying him out, essentially causing a DoS. Proof of Concept Lender uses an upgradeable smart contract that front runs competing lenders and upgrades...

Burning collateralContractAddress by mistake in closeLoan

Lines of code closeLoan; L116-216 Vulnerability details Impact ERC721 used as collateral could possibly never return to borrower. Proof of Concept No zero address check for sendCollateralTo might lead to sending ERC721 used as collateral to inexistent address. Use of transferFrom instead of...

Malicious users can frontrun borrowers trying to repay loans, causing DoS and possibly a loan default

Lines of code Vulnerability details Impact Attackers can listen for a borrower to call repayAndCloseLoan on a specific loanId, and frontrun their transaction with a call to lend, creating a new loan with an increased amount, causing the borrower's transaction to fail due to the new loanAmount bei...

Borrower can be their own lender and steal funds from buyout due to reentrancy

Lines of code Vulnerability details Impact If borrower lends their own loan, they can repay and close the loan before ownership of the lend ticket is transferred to the new lender. The borrower will keep the NFT + loan amount + accrued interest. Proof of Concept This exploit requires that the...

Using transferFrom on ERC721 tokens transfer

Lines of code Vulnerability details Impact In the function closeLoan of contract NFTLoanFacilitator, collateral ERC721 token is sent to sendCollateralTo, the transferFrom keyword is used instead of safeTransferFrom. If any sendCollateralTo is a contract and is not aware of incoming ERC721 tokens,...

Protocol doesn't handle fee on transfer tokens

Lines of code Vulnerability details Impact Since the borrower is able to specify any asset token, it is possible that loans will be created with tokens that support fee on transfer. If a fee on transfer asset token is chosen, the protocol will contain a point of failure on the original lend call...

When an attacker lends to a loan, the attacker can trigger DoS that any lenders can not buyout it

Lines of code Vulnerability details Impact If an attacker lender lends to a loan, the attacker can always revert transactions when any lenders try to buyout, making anyone can not buyout the loan of the attacker. Proof of Concept 1. A victim calls lend, trying to buyout the loan of the attacker. ...

Code credits fee-on-transfer tokens for amount stated, not amount transferred

Lines of code Vulnerability details Some ERC20 tokens, such as Tether USDT, allow for charging a fee any time transfer or transferFrom is called. Impact The code miscalculates whether the loan has been fully paid or bought out because it relies on the value of amounts passed in rather than the...

Malicious owner can steal some funds from borrower

Lines of code Vulnerability details Impact Owner can make changes to the protocol with immediate effect. Malicious owner can watch for big lend in the mempool and front run it by maxing out originationFeeRate to 5%. The users, both lender and borrower, will still think that originationFeeRate is...

Using transferFrom on ERC721 tokens

Lines of code Vulnerability details In the function closeLoan of contract NFTLoanFacilitator.sol, the transferFrom keyword is used instead of safeTransferFrom. If the arbitrary sendCollateralTo address is a contract and is not aware of the incoming ERC721 token, the sent token could be locked. I...

Use safeTransferFrom() consistently instead of transferFrom()

Lines of code Vulnerability details Impact While the contracts use in most places safeTransferFrom to transfer NFTs, there are a few cases where the unsafe counterpart transferFrom is used. safeTransferFrom checks that contract recipients are aware of the ERC721 protocol to prevent tokens from...

Lend and borrow tickets can be transferred to the NFTLoanFacilitator

Lines of code Vulnerability details Impact Both lend and borrow tickets can be intentionally or accidentally transferred to the NFTLoanFacilitator contract. Since the NFTLoanFacilitator has no mechanism for rescuing these tokens or preventing their transfer, borrowers may be unable to repay,...