Options can be sold for tokens that have been destructed

Lines of code Vulnerability details Unlike OpenZeppelin's safeTransfer, solmate's version of the function does not check for contract existence before its low level calls are made. The foot-gun associated with this difference is that it's up to the caller to verify that the contract exists before...

A malicious user can create a vault that is actually empty

Lines of code Vulnerability details description A malicious user can call the createVault function to create a vault with an ERC20 token that returns false rather than revert on failed transfer. By specifying the tokenType parameter to be ERC721 the transferFrom function will be called rather tha...

Re-entrancy attack on the main functions

Lines of code Vulnerability details Impact A malicious token, or one that implemented transfer hooks, could re-enter the public calling function such as withdraw before proper internal accounting was completed. Because the earned reward function looks up the pool.totalDepositsWei and...

Missing freshness validation in ETH price oracle

Originally submitted by warden horsefacts in 199, duplicate of 17. Missing freshness validation in ETH price oracle The ChainlinkUsdWrapperethPrice function does not check for a nonzero answer or validate that the price was returned in a recent round: ChainlinkUsdWrapperethPrice function ethPrice...

Users will pay more than required for NFT Minting

Lines of code Vulnerability details Impact NFTs should be sold for their best price without descending into a gas race: Due to SpeedBumpPriceGate.sol function passThruGate code users will pay more than required for NFT Minting. User will pay msg.value and not the NFT "price". It is very likely th...

Admin can manipulate nft prices by _setNftPriceOracle

Lines of code Vulnerability details impact setNftPriceOracle is an admin-only function which can set a new nftOracle. But there is no restriction in this function. An admin can set any oracle unconditionally. It could be used to manipulate nft prices in liquidateCalculateSeizeNfts. Proof of Conce...

Deprecated safeApprove() function

Originally submitted by warden Dravee in 146, duplicate of 178 related to the use of safeApprove. This is upgraded from a QA report to standalone issue because it correctly described the revert when trying to call safeApprove on non-zero allowance. QA report that only describe safeApprove as...

COMP Distributions Can Be Manipulated And Duplicated Across Any Number Of Accounts

Lines of code Vulnerability details Impact The updateCompSupplyIndex and distributeSupplierComp functions are used by Compound to track distributions owed to users for supplying funds to the protocol. Bunker protocol is a fork of compound with NFT integration, however, part of the original...

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

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

A malicious actor can cause DoS with Block Gas Limit and destroy the sale flow also having advantage of buying cheaper price

Lines of code Vulnerability details Impact A malicious actor can cause DoS with Block Gas Limit and mint NFT's on cheaper price as the price drop is in action or destroy the sale flow. Proof of Concept Each block has an upper bound on the amount of gas that can be spent, and thus the amount...

reentryattack with owner checks and effecst pattern not implemented

Lines of code Vulnerability details Impact reentryattack can happen because checks and effects pattern not implemented Proof of Concept a owner can be millousius and take advantage of effects after a call Recommended Mitigation Steps do the checks and effects pattern --- The text was updated...

Limit not enforced on teamSummon function

Lines of code Vulnerability details Impact The docs say there is a cap on how many tokens the project team can mint, however there are no checks or tracking implemented in the teamSummon function to enforce that limit. An admin calling that function could accidentally or maliciously exceed the...

Missing validations in setter and setPhaseTimes() functions.

Lines of code Vulnerability details Impact Time variable validations daStartTime, mintlistStartTime, publicStartTime, claimsStartTime, selfRefundsStartTime are only done in setPhaseTimes and not even all validations. These validations should be performed: daStartTime block.timestamp...

Impossible to liquidate small amounts

Lines of code Vulnerability details Impact It might not be possible to liqudate small positions depending on the liquidateCallerReward. Function liquidate in PARMinerV2 first transfers liquidateCallerReward to the liquidator and then expects that the remaining par balance is greater than it was...

SuperVault's leverageSwap and emptyVaultOperation can become stuck

Lines of code Vulnerability details leverageSwap and emptyVaultOperation can be run repeatedly for the same tokens. If these tokens happen to be an ERC20 that do not allow for approval of positive amount when allowance already positive, both functions can become stuck. In both cases logic doesn't...

Fund loss or theft by attacker with creating a flash loan and setting SuperVault as receiver so executeOperation() will be get called by lendingPool but with attackers specified params

Lines of code Vulnerability details Impact According to Aave documentation, when requesting flash-loan, it's possible to specify a receiver, so function executeOperation of that receiver will be called by lendingPool. In the SuperVault there is no check to prevent this attack so attacker can use...

ChainlinkInceptionPriceFeed can report stale price

Lines of code Vulnerability details As stale price is determined by time since last timestamp, the price that is most recent, but wasn't updated for more than PRICEORACLESTALETHRESHOLD say there were no trades on the market will be rejected, which makes system unavailable in such a case. This can...

Use Victims Collateral by Frontrunning

Lines of code Vulnerability details Issue: the require statement in requestLoan doesn't check if the one that provides the collateral is the same address that requests the loan Consequences: If an NFT is transferred to the contract by itself, either on accident or preparing to call requestLoan wi...

Oracle failure allows NFT to be stolen

Lines of code Vulnerability details Impact Any temporary failure in an oracle relaying a price allows the NFT collateral to be removed by the lender, even if the value of the NFT is still far above the agreed-upon liquidation value. Considering that oracle price retrieval failure is accounted for...

[WP-M1] supplyTokenTo() may fail when Aave Pool address changed

Lines of code Vulnerability details function supplyTokenTouint256 depositAmount, address to external override nonReentrant uint256 shares = tokenToSharesdepositAmount; requireshares 0, "AaveV3YS/shares-gt-zero"; address underlyingAssetAddress = tokenAddress;...

Lender can adjust ltvBPS and instantly sieze collateral

Lines of code Vulnerability details Impact After the loan is accepted and is in the LOANOUTSTANDING phase, updateLoanParams allows the lender to update the TokenLoanParams.ltvBPS parameter. The require check allows for the parameters to change as long as they are favorable for the borrower, but...

Calculation without check may result in tiny loss of user funds

Lines of code Vulnerability details Impact Calculation without the bigger than zero check may result in loss of user funds, albeit in tiny amounts as of now. Proof of Concept In this line of redeemToken shares to burn is calculated through tokenToShares method . As there is no check that checks i...

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

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

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

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

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

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

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

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

Swapper3Crv.sol use the wrong address for sushiswap

Lines of code Vulnerability details Impact This seems to be out of scope, but still worth mentioning. This line should be address sushiSwap = SUSHISWAP; --- The text was updated successfully, but these errors were encountered: All reactions...

Lack of safeApprove(0) prevents some registrations, and the changing of stakers and LP tokens

Lines of code Vulnerability details OpenZeppelin's safeApprove will revert if the account already is approved and the new safeApprove is done with a non-zero value function safeApprove IERC20 token, address spender, uint256 value internal // safeApprove should only be called when setting an initi...

BufferStored could be larger than BufferCap after _setBufferCap()

Judge @jack-the-pug has assessed the second item in QA Report 64 as Medium risk. The relevant finding follows: Impact In RateLimited.sol BufferCap should be the upper bound of BufferStored, However in setBufferCap it calls updateBufferStored before replacing the old BufferCap. If old BufferCap is...

function lockFunds in TopUpActionLibrary can cause serious fund lose. fee and Capped bypass. It's not calling stakerVault.increaseActionLockedBalance when transfers stakes.

Lines of code Vulnerability details Impact In function TopUpActionLibrary.lockFunds when transfers stakes from payer it doesn't call stakerVault.increaseActionLockedBalance for that payer so stakerVault.actionLockedBalancespayer is not get updated for payer and...

Potentially Incomplete or Stale Data from Oracle

Lines of code Vulnerability details Impact Calls to the Chainlink price oracle via refreshedAssetPerBaseInUQ in ChainlinkPriceOracle.sol use the correct function latestRoundData per Chainlink's documentation, but lacks the recommended validations to ensure that the round is complete and does not...

ORACLE Data is not properly validated in ChainlinkPriceOracle.sol

Lines of code Vulnerability details Impact Price can be stale which can lead to wrong assetPerBaseInUQ return value Proof of Concept Oracle data feed is insufficiently validated. There is no check for stale price and round completeness. Tools Used Manual review, similar issue was found in yield...

Index mint and burn calls can be front run

Lines of code Vulnerability details Impact Both in the mint and burn cases all the user supplied / due to a user assets can be stolen by an attacker, who detects correspondingly asset transfer calls / Index token transfer call and front runs Index contract's mint / burn call with own address as a...

Chainlink's latestRoundData missing security checks

Lines of code Vulnerability details Impact Protocol uses Chainlink as one of the oracles that provides prices for the assets. Chainlink's latestRoundData is used but the implementation is missing important security checks that can result in stale and incorrect prices being returned. Proof of...

Staked Citadel function balance and reportHarvest

Lines of code Vulnerability details Impact In the StakedCitadel...

Governor Infinitely Extend To Vesting Period

Lines of code Vulnerability details Impact During the review of StakedCitadelVester contract, It has been noticed governor role can extend vesting period indefinitely. There is no upper bound defined in the vesting period. Although, It is dependent on the governor, If governor private key is...

latestRoundData data insufficiently validated

Lines of code Vulnerability details Impact The data returned by the Chainlink latestRoundData function may be stale. There should be checks applied on the data received from Chainlink to validate that it is not stale. Proof of Concept The ChainlinkPriceOracle contract has these two lines , int...

New YVault depositors can be attacked by depressing share decimals

Lines of code Vulnerability details Impact An attacker can become the first depositor for a recently created YVault contract, providing a tiny amount of token by calling deposit1 raw values here, 1 is 1 wei, 1e18 is 1 token if it is 18 decimals. Then the attacker can directly transfer, for exampl...

Use of deprecated Chainlink oracle aggregator function latestAnswer

Lines of code Vulnerability details Impact According to Chainlink's documentation, the latestAnswer function is deprecated. This function does not error if no answer has been reached but returns 0. The function is not present in the latest API reference AggregatorInterfaceV3. Proof of Concept...

LPFarming, yVault and yVaultLPFarming contract white listing can be surpassed

Lines of code Vulnerability details Impact LPFarming, yVault and yVaultLPFarming contracts' white list checks are performed with isContract and can be surpassed. isContract can only be used for positive confirmations, i.e. filtering out EOAs. Setting high severity as it is a direct access control...

StrategyPUSDConvex.balanceOfJPEG uses incorrect function signature while calling extraReward.earned, causing the function to unexpectedly revert everytime

Lines of code Vulnerability details Impact As specified in Convex BaseRewardPool.sol and VirtualRewardPool.sol, the function signature of earned is earnedaddress. However, balanceOfJPEG did not pass any arguments to earned, which would cause balanceOfJPEG to always revert. This bug will propagate...

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

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

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

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

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