Borrower can frontrun lender's call to lend to change the price oracle

Lines of code Vulnerability details Impact In NFTPairWithOracle.lend, params.oracle is not checked. This allow a borrower to watch the mempool and front-run the lender's call and change oracle to avoid liquidation. Proof of Concept function lend address lender, uint256 tokenId, TokenLoanParams...

Params.oracle can be changed to a fake one by front running NFTPairWithOracle's lend function

Lines of code Vulnerability details Impact As neither lend, nor updateLoanParams functions verify params.oracle, the lend call can be front run by a malicious borrower with changing params.oracle to a non-market one. The front run will be an updateLoanParams call where borrower introduces fake...

Malicious lender can change price oracle for outstanding loan

Lines of code Vulnerability details The updateLoanParams function in NFTPairWithOracle.sol allows the lender to update parameters for an outstanding loan duration, valuation, annual interest, and collateralization ratio as long as they are the same or better for the borrower. These params are...

NFT oracle price request successful or not is not checked

Lines of code Vulnerability details Impact Loan can be destroyed because of no price reported by oracle. or any other reason that makes oracle "work as expected" in bad situations when the get function return success=false Proof of Concept According to the interface of INFTOracle, the first...

updateLoanParams can update to any malicious oracle address

Lines of code Vulnerability details Impact A lender attacker can modify the oracle address of tokenLoanParams to get collateral directly before expiration. Proof of Concept 1. First a lender attacker lends for a loan, the attacker can use updateLoanParams function and update to a malicious oracle...

Loss of Collateral Via Malicious Oracle

Lines of code Vulnerability details Issue: The oracle can be arbitrarily updated at any point in time by the lender. Consequences: A lender can inject a malicious oracle at any time and steal the collateral NFT at the cost of his loaned tokens. Proof of Concept Borrower requests loan with an hone...

Borrower can frontrun lender's call to lend to increase ltvBPS and avoid liquidation

Lines of code Vulnerability details Impact In NFTPairWithOracle.lend, the loan ltvBPS can be higher than the lender's accepted ltvBPS. This allow a borrower to watch the mempool and front-run the lender's call and change ltvBPS to some very large value using updateLoanParams to avoid liquidation...

No success required for Oracle market rate queries

Lines of code Vulnerability details Impact The system can use stale or even plainly incorrect due to any technical malfunction price for decision making. For example, a malicious lender can setup a bot that tracks incorrect readings i.e. track the state of the Oracle used and act on observing...

NFTPairWithOracle's _lend ignores accepted.oracle and allows to start loan with empty params.oracle

Lines of code Vulnerability details Impact As lend doesn't require params.oracle to be valid, while removeCollateral does, the loan initiation with an empty oracle can lead to ignoring collateral valuation. As the deals are OTC this can be seen as lender decision. However, lend ignores...

Loss Of Collateral Via Illegitimate Liquidation

Lines of code Vulnerability details Issue: updateLoanParams allows the lender to change the terms of an in-progress loan to lower ltvBPS. removeCollateral calculates whether liquidation is allowed via requirerate.mulloanParams.ltvBPS / BPS amount, "NFT is still valued";. A low or 0 ltvBPS...

Unchecked oracle return value

Lines of code Vulnerability details Impact The return value bool success of oracle.get calls is ignored. This could lead to stale data or incorrect prices due to oracle issues. Proof of Concept NFTPairWithOracle.solL287 Change to bool success, uint256 rate = loanParams.oracle.getaddressthis,...

Pool has free access to all funds without using the redeem method

Lines of code Vulnerability details Impact Pool has access to all funds even if they weren't deposited by the pool Proof of Concept The AAVE yield source contract allows anyone to deposit and withdraw funds that are deposited or withdrawn from AAVE. In the constructor of the contract, max approva...

Inconsistent balance when supplying fee-on transfer tokens

Lines of code Vulnerability details Impact There are ERC20 tokens that may make certain customizations to their ERC20 contracts. One type of these tokens is deflationary tokens that charge a certain fee for every transfer or transferFrom. Proof of Concept AaveV3YieldSource.solL237 Tools Used Manu...

[WP-H1] A malicious early user/attacker can manipulate the vault's pricePerShare to take an unfair share of future users' deposits

Lines of code Vulnerability details This is a well-known attack vector for new contracts that utilize pricePerShare for accounting. / @notice Calculates the number of shares that should be minted or burnt when a user deposit or withdraw. @param tokens Amount of asset tokens @return Number of...

Contract Can Be Bricked by Transferring a Large Amount of aTokens

Lines of code Vulnerability details Impact The supplyTokenTo function is executed when a yield source prize pool deposits tokens into Aave to generate yield. This contract uses an internal function called tokenToShares to calculate how many shares to mint on behalf of the to account. Because this...

Yield source does not correctly calculate share conversions

Lines of code Vulnerability details The aTokens’ value is pegged to the value of the corresponding supplied asset at a 1:1 ratio and can be safely stored, transferred or traded. All yield collected by the aTokens' reserves are distributed to aToken holders directly by continuously increasing thei...

Lender can immediately liquidate valued Collateral in NFTPairWithOracle contract

Lines of code Vulnerability details Impact A lender can liquidate a borrower's collateral immediately by calling updateLoanParams in the NFTPairWithOracle contract where the ltvBPS for the params struct is set to 0. This bypasses the checks to make sure that the terms are favourable to the...

Borrower can update loan parameters and force lender into accepting near-unliquidatable loan

Lines of code Vulnerability details Impact While loan is in LOANREQUESTED phase, the updateLoanParameters function can be called by the borrower to adjust TokenLoanParams.ltvBPS to a value that is very unfavorable for a prospective lender. Usually, this results in the loan not being accepted by a...

Protocol fees during origination are based on another fee rather than on the loan amount

Lines of code Vulnerability details Impact Protocol fee revenue will be much lower than expected. If the average NFT loaned on the platform is worth $900, openFeeShare will be $9, and protocolFeeShare will be only $0.90. There would have to be more than 50,000 such loans to cover the cost of this...

Outstanding loan Oracle can be arbitrary changed by a lender

Lines of code Vulnerability details Impact As updateLoanParams function do not verify params.oracle, a lender for an already outstanding loan can change params.oracle to a non-market one. For example, the lender can set oracle to a pre-cooked INFTOracle contract reporting 0 price of the asset and...

Reentrancy at _requestLoan allows requesting a loan without supplying collateral

Lines of code Vulnerability details requestLoan makes an external call to the collateral contract before updating the NFTPair contract state. Impact If the ERC721 collateral has a afterTokenTransfer hook, The NFTPair contract can be reentered, and a loan can be requested without the borrower...

User fund loss in supplyTokenTo() because of rounding

Lines of code Vulnerability details Impact When user use supplyTokenTo to deposit his tokens and get share in FeildSource because of rounding in division user gets lower amount of share. for example if token's decimal was 1 and totalSupply was 1000 and aToken.balanceOfFieldSource.address was 2100...

Fund theft in redeemToken() because of rounding in division

Lines of code Vulnerability details Impact When user use redeemToken to get his tokens and burn his shares in FeildSource because of rounding in division user's share balance decrease is going to be lower than corresponding withdraw amount in value. for example if token's decimal was 1 and...

User fund lose and DOS attack when totalSupply() is zero

Lines of code Vulnerability details Impact Hacker can do this two action: Perform a DOS attack and continuously deny users from supplying their tokens to YeildSource deposits less than a high amount will be rejected and attacker can control this amount, for example set it to 10K and every deposit...

Early Depositor can DOS Deposits

Lines of code Vulnerability details Impact A malicious, but generous, early depositor can DOS all future deposits. This is accomplished by directly sending aTokens to the AaveV3YieldSource.sol contract after making their first deposit. The amount of aTokens sent to the contract will manipulate th...

Moral hazard of borrower calling liquidate() and potential Oracle manipulation

Lines of code Vulnerability details Impact In the InceptionVaultsCore contract, the liquidate and liquidatePartial function can be called anyone. This means that the borrower for a specific vaultId can call liquidate or liquidatePartial on his own vault. Furthermore, the project incentivizes...

Unsupported fee-on-transfer tokens

Low-01 Unsupported fee-on-transfer tokens Impact When underlyingAssetAddress is fee-on-transfer tokens, in the supplyTokenTo function, the actual amount of tokens received by the contract will be less than the depositAmount, so that the subsequent pool.supply function will fail to execute. Proof ...

Manager or owner can send rewards to any address

Lines of code Vulnerability details Impact In the claimRewards function, manager or owner can send rewards to any address. function claimRewardsaddress to external onlyManagerOrOwner returns bool requireto != address0, "AaveV3YS/payee-not-zero-address"; address memory assets = new address; assets...

Variables that should be bounded

Judge @GalloDaSballo has assessed the 8th item in QA Report 263 as Medium risk. The relevant finding follows: … The variable MasterChef.sol:43: uint16 depositFeeBP; // Deposit fee in basis points is never bounded, and UInt16.MaxValue is 65535 --- The text was updated successfully, but these error...

You should use safe casts here

Judge @GalloDaSballo has assessed the 6th item in QA Report 254 as Medium risk. The relevant finding follows: … You should use safe casts here: depositspidmsg.sender.amount += uint192amount; depositspidmsg.sender.amount -= uint192amount; amount : uint192amount Otherwise, if token amounts are...

Function recoverERC20 in StakingRewards allows an owner to transfer out any token except stakingToken

Judge @GalloDaSballo has assessed the 1st item in QA Report 254 as Medium risk. The relevant finding follows: … Function recoverERC20 in StakingRewards allows an owner to transfer out any token except stakingToken. I see 2 problems with this: 1. It should also forbid transferring of rewardsToken,...

Missing Re-entrancy Guard

Judge @GalloDaSballo has assessed item C4-009 in QA Report 198 as Medium risk. The relevant finding follows: … Impact - Non-Critical Consider using ReentrancyGuard to protect functions that have external calls and do not follow Checks Effects Interactions pattern. An example of a function that...

Users' funds can become locked in the contract

Judge @GalloDaSballo has assessed the 2nd item in QA Report 163 as Medium risk. The relevant finding follows: … Users' funds can become locked in the contract In the withdraw function in ConvexStakingWrapper there's a call to MasterChef's withdraw funciton which uses the safeConcurTransfer functi...

claimRewards() didnt follow the safe check effect pattern

Judge @GalloDaSballo has assessed the 2nd item in QA Report 230 as Medium risk. The relevant finding follows: … Impact a user can claim a reward by calling the claimRewards, however this function didnt follow the correct check effect pattern, where the zero address is set after making an external...

withdrawFees() function shoud require to address to not be zero

Lines of code Vulnerability details Impact withdrawFees don't check that to address is not zero and send fee to the address without any check that confirms admin has set the address. bentoBox don't accept transferring to zero address, otherwise this could be high risk Proof of Concept As you can...

depositAmount requires to be updated to contract balance increase (6)

Lines of code Vulnerability details Impact Every time transferFrom or transfer function in ERC20 standard is called there is a possibility that underlying smart contract did not transfer the exact amount entered. It is required to find out contract balance increase/decrease after the transfer. Th...

When the owner uses the cloneInceptionVault function to create an InceptionVault, he can control the oracle address

Lines of code Vulnerability details Impact The owner can control the oracle address when creating an InceptionVault using the cloneInceptionVault function of the InceptionVaultFactory contract. A malicious owner can use create2 to deploy a normal oracle, and then use the oracle's address to creat...

Reentrency in claimRewards in ConcurRewardPool

Judge @GalloDaSballo has assessed the 1st item in QA Report 163 as Medium risk. The relevant finding follows: … Reentrency in claimRewards in ConcurRewardPool The function claimRewards is open to reenterncy, if the safeTransfer function of a token calls the claimRewards again the tokens can be...

Lack of require in redeemToken could produce token loses

Lines of code Vulnerability details Impact Lack of require in redeemToken could produce token loses. Proof of Concept In the method redeemToken the user set the expected redeemAmount, it will compute the expected shares to burn, and after it, it will transfer the amout according the redeemAmount...

amount requires to be updated to contract balance increase (5)

Lines of code Vulnerability details Impact Every time transferFrom or transfer function in ERC20 standard is called there is a possibility that underlying smart contract did not transfer the exact amount entered. It is required to find out contract balance increase/decrease after the transfer. Th...

_depositAmount requires to be updated to contract balance increase

Lines of code Vulnerability details Impact Every time transferFrom or transfer function in ERC20 standard is called there is a possibility that underlying smart contract did not transfer the exact amount entered. It is required to find out contract balance increase/decrease after the transfer. Th...

_amount requires to be updated to contract balance increase (2)

Lines of code Vulnerability details Impact Every time transferFrom or transfer function in ERC20 standard is called there is a possibility that underlying smart contract did not transfer the exact amount entered. It is required to find out contract balance increase/decrease after the transfer. Th...

Potential Sandwich Attack: Arbitrage bots can front run reward tokens being sent to the liquidity mining contracts

Lines of code Vulnerability details Impact For the PARMiner and DemandMiner contracts, arbitrage bots could harvest significant portion of rewards by monitoring MEV, and front run any reward token either a.mimo or par being transferred to the liquidityMining contract i.e. call the deposit functio...

_amount requires to be updated to contract balance increase (4)

Lines of code Vulnerability details Impact Every time transferFrom or transfer function in ERC20 standard is called there is a possibility that underlying smart contract did not transfer the exact amount entered. It is required to find out contract balance increase/decrease after the transfer. Th...

Improper Upper Bound Definition on the depositFeeBP

Judge @GalloDaSballo has assessed item C4-002 in QA Report 198 as Medium risk. The relevant finding follows: … Impact - LOW The add function does not have any upper or lower bounds. Values that are too large will lead to reversions in several critical functions. User funds will be locked forever...

_amount requires to be updated to contract balance increase (3)

Lines of code Vulnerability details Impact Every time transferFrom or transfer function in ERC20 standard is called there is a possibility that underlying smart contract did not transfer the exact amount entered. It is required to find out contract balance increase/decrease after the transfer. Th...

Shelter donations result in wrong internal balance if fee-on-transfer token is used

Judge @GalloDaSballo has assessed the 2nd Low-severity item in QA Report 66 as Medium risk. The relevant finding follows: … In case of a fee-on-transfer ERC20 token the savedTokens balance will not represent the actual balance of the contract: You can either disable the use of fee-on-transfer...

Reward lost

Judge @GalloDaSballo has assessed the 11th item in QA Report 26 as Medium risk. The relevant finding follows: … Contract: In notifyRewardAmount function, if Admin added a reward 100 once block.timestamp = periodFinish. Now if Admin decides to add 200 rewards calling this function at block.timesta...

Potential reentrance in claimRewards

Judge @GalloDaSballo has assessed the 1st item in QA Report 36 as Medium risk. The relevant finding follows: … POC IERC20tokensi.safeTransfermsg.sender, getting; rewardmsg.sendertokensi = 0; Considering there are exterTokens, it is possible that some token will provide reentry opportunities...

Cast to uint192 is unsafe

Judge @GalloDaSballo has assessed the 3rd item in QA Report 197 as Medium risk. The relevant finding follows: … Cast to uint192 is unsafe, user trying to deposit more than 2^192 would recevie less deposits balance. depositspidmsg.sender.amount += uint192amount; --- The text was updated...