Use of deprecated oracle API in _normalizeAggregatorAnswer

Lines of code Vulnerability details Use of deprecated oracle API in normalizeAggregatorAnswer Likelihood low, impact high. The Chainlink latestAnswer function included in IAggregatorV3Interface and called in NFTVaultnormalizeAggregatorAnswer is considered deprecated and no longer included in the...

JPEGLock lockups overwrite previous data

Lines of code Vulnerability details Impact If a user calls NFTVault.finalizePendingNFTValueETH a second time without first calling JPEGLock.unlock to recover their previous lockup, their balance will be overwritten leaving the previous lockup balance unrecoverable. Proof of Concept POC by adding...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

currentLoanOwner can manipulate loanInfo when any lenders try to buyout

Lines of code Vulnerability details Impact If an attacker already calls lend to lend to a loan, the attacker can manipulate loanInfo by reentrancy attack when any lenders try to buyout. The attacker can set bad values of lendInfo e.g. very long duration, and 0 interest rate that the lender who...

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

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

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

Consistently check account balance before and after transfers for Fee-On-Transfer discrepancies

Lines of code Vulnerability details Impact Wrong amount calculated for facilitatorTake Proof of Concept Arbitrary ERC20 tokens can be passed as loanAssetContractAddress. With a transfer, the received amount should be calculated every time to take into consideration a possible fee-on-transfer or...

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

Loan buyouts can stick users with huge surprise bills

Lines of code Vulnerability details Impact If a small loan is bought out with a new loanAmount in the millions, so much interest can be charged that the borrower is unable to pay back the loan, essentially guaranteeing whales will be able to Seize NFTs. Proof of Concept The only requirement as it...

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

sendCollateralTo is unchecked in closeLoan(), which can cause user's collateral NFT to be frozen

Lines of code Vulnerability details function closeLoanuint256 loanId, address sendCollateralTo external override notClosedloanId requireIERC721borrowTicketContract.ownerOfloanId == msg.sender, "NFTLoanFacilitator: borrow ticket holder only"; Loan storage loan = loanInfoloanId;...

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

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

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