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

Initial yVault deposit with amount=1 wei causes very expensive share price leading to precision errors and loss of funds

Lines of code Vulnerability details Impact The first depositor into yVault is able to maliciously manipulate the share price by depositing the lowest possible amount 1 wei and then artificially blowing up the yVault token balance. Following depositors will loose their deposited funds due to...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Loan can not be repaid when the BorrowTicket's owner is a contract with no onERC721Received method

Lines of code Vulnerability details In repayAndCloseLoan, after the loan is repaid, the Collateral NFT will safeTransferFrom addressthis to the owner of the BorrowTicket NFT, which is the mintBorrowTicketTo used in createLoan, must implement the onERC721Received method for the safeTransferFrom to...

requiredImprovementRate can not work as expected when previousInterestRate less than 10 due to precision loss

Lines of code Vulnerability details uint256 previousInterestRate = loan.perAnumInterestRate; uint256 previousDurationSeconds = loan.durationSeconds; requireinterestRate = previousDurationSeconds, 'NFTLoanFacilitator: duration too low'; requirepreviousLoanAmount requiredImprovementRate / SCALAR =...

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

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

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

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

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

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

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

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

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

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