Pools and trees may be underfunded for fee-on-transfer tokens

Lines of code Vulnerability details Pools, vesting trees, and airdrop trees may all be created with fee-on-transfer tokens. When each of these entities is funded by a transfer in, their internal accounting assumes they receive the full amount transferred. However, they may actually receive fewer...

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

Use of deprecated Chainlink function latestAnswer

Lines of code Vulnerability details Impact Use of deprecated Chainlink function latestAnswer According to Chainlink's documentation, the latestAnswer function is deprecated. This function does not error if no answer has been reached but returns 0, causing an incorrect price feed to USDC Price...

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

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

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

Unbound loop can brick contract and freeze miner payments

Lines of code Vulnerability details Impact The payees array in the MinerPayer.sol contract can grow to an unbounded size. That means it could grow so large that the MinerPayer.release function can't loop through them all without triggering an out of gas error. Updating the payees array in the...

VaultCore's withdrawETH uses transfer function, which can be unusable for some smart contracts

Lines of code Vulnerability details Impact When a user wishes to withdraw their collateral from a WETH vault, the funds are returned to the user with msg.sender.transfer. Using transfer is not recommended anymore, especially for critical operations like withdrawing collateral from a vault as it...

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

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

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

[WP-H8] Special ERC721 compatible implementation may allow an attacker to requestLoan without transferring in the NFT collateral

Lines of code Vulnerability details NFT is a fragmented standard, for certain non-standard ERC721 implementations, they may have built-in hooks that can be used to re-enter the contract. Just like ERC777 to ERC20. For example, if the collateral NFT got a pre-transfer hook to the receiver of the...

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

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

setFlywheelRewards can take any rewardToken

Lines of code Vulnerability details Impact Though setFlywheelRewards has requiresAuth, it still has rug risk that a privileged user can move all rewardToken of flywheelRewards to new malicious newFlywheelRewards unconditionally. Proof of Concept A malicious user or a compromised admin can call...

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

Transfer return value is ignored

Impact Some ERC20 tokens, such as USDT, don't revert when transfer/transferFrom fails. The transfer return value has to be checked as there are some other tokens that returns false instead revert. safeTransfer should be used instead of transfer Proof of Concept safeTransferFrom should be used...

Chainlink oracle data can be stale

Lines of code Vulnerability details Impact Oracle data can be stale which can lead to wrong calculations for balancing indexes. Proof of Concept When lastRoundData is called only price is pulled from the provided data. RoundId should be checked to ensure the data is updated. Recommended Mitigatio...

StakedCitadelVester/claimableBalance() has problems when users keep vesting.

Lines of code Vulnerability details Impact When users vest in StakedCitadelVester, the tokens will be locked in vest duration. Users can call claim to get back their tokens. The claimable amount is calculated in claimableBalance. Before the duration ends, the claimable amount is locked...

Staked Citadel function balance and reportHarvest

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

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

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

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

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

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

NFTLoanFacilitator: Unsupported fee-on-transfer tokens

Lines of code Vulnerability details Impact loanAssetContractAddress is specified by the user when the user creates a loan. When loanAssetContractAddress is fee-on-transfer tokens, in the lend function, the actual amount of tokens received by the contract will be less than the amount, so that the...

Users Can Bypass Emergency Restrictions on updateUserRewardState()

Lines of code Vulnerability details Impact The emergencyWithdraw function intends to withdraw their tokens regardless if they are locked up for any duration. This emergency must be triggered by the owner of the contract by calling triggerEmergencyWithdraw. A number of functions will revert when t...

Royalties do Not Support Fee-On-Transfer Tokens

Lines of code Vulnerability details Impact The RoyaltyVault.sol contract interacts with the Splitter.sol to send accumulated royalties to the collection's respective recipients. The sendToSplitter function will query the balance of the royalty asset and send the amount after fee deductions to the...

admin can rug

Lines of code Vulnerability details In the function transferToken, the admin can steal all the money. --- The text was updated successfully, but these errors were encountered: All reactions...

Cooldown tricking possible

Lines of code Vulnerability details Impact Due to an approach that upon a transfer the weighted average is calculated for the cooldown, it is possible to trick it in a way that ready funds can "thrive" in the allowed window, ready to payout. Anytime it is ending, those funds can be "mixed" with...

Add a timelock to PaladinRewardReserve functions

Lines of code Vulnerability details Impact The owner of PaladinRewardReserve can approve and transfer any amount of tokens with no limits on any account. This is not good for investors. To give more trust to users: these functions should be put behind a timelock. Proof of Concept Tools Used VS Co...

Not handling return value of transferFrom command can create inconsistency

Lines of code Vulnerability details The below transferFrom command is called at two places in the core contracts, followed by an emit event payableToken.transferFrommsg.sender,recipient,amount emit ......; The return value is not checked during the payableToken.transferFrom Impact In the event of...

A large platformFee (>10000), would cause underflow during sendToSplitter (at RoyaltyVault.sol)

Lines of code Vulnerability details Impact at RoyaltyVault.sol Presently platformFee, does not have a upper limit and can be set to any value through setPlatformFee function. If the value is set beyond 10,000 it would cause an underflow during split share calculation at sendToSplitter function...

Add a timelock to setPlatformFee()

Lines of code Vulnerability details Impact It is a good practice to give time for users to react and adjust to critical changes. A timelock provides more guarantees and reduces the level of trust required, thus decreasing risk for users. It also indicates that the project is legitimate. Here, no...

Improper Upper Bound Definition on the Fee

Lines of code Vulnerability details Impact The platformFee does not have any upper or lower bounds. Values that are too large will lead to reversions in several critical functions or the platform user will lost all funds when paying the fee. Proof of Concept 1. Navigate to the following contract...

No reentrancy guard on mint() function that has a callback

Lines of code Vulnerability details Impact the mint function calls mint which has a callback to the "to" address argument. Functions with callbacks should have reentrancy guards in place for protection against possible malicious actors both from inside and outside the protocol. Proof of Concept...

DoS: Attacker May Front-Run createSplit() With A merkleRoot Causing Future Transactions With The Same merkleRoot to Revert

Lines of code Vulnerability details Impact A merkleRoot may only be used once in createSplit since it is used as salt to the deployment of a SplitProxy. The result is an attacker may front-run any createSplit transaction in the mem pool and create another createSplit transaction with a higher gas...

QTokens with the same symbol will lead to mistakes

Lines of code Vulnerability details The README.md states: Bob can then trade the QToken with Alice for a premium. The method for doing that is beyond the scope of the protocol but can be done via any smart contract trading platform e.g. 0x. It is therefore important that tokens be easily...

[WP-H6] Admin of the upgradeable proxy contract of Controller.sol can rug users

Lines of code Vulnerability details Use of Upgradeable Proxy Contract Structure allows the logic of the contract to be arbitrarily changed. This allows the proxy admin to perform malicious actions e.g., taking funds from users' wallets up to the allowance limit. This action can be performed by th...

[WP-H2] EIP712MetaTransaction.executeMetaTransaction() failed txs are open to replay attacks

Lines of code Vulnerability details Any transactions that fail based on some conditions that may change in the future are not safe to be executed again later e.g. transactions that are based on others actions, or time-dependent etc. In the current implementation, once the low-level call is failed...

Mint spread collateral-less and conjuring collateral claims out of thin air with implicit arithmetic rounding and flawed int to uint conversion

Lines of code Vulnerability details Impact This report presents 2 different incorrect behaviour that can affect the correctness of math calculations 1. Unattended Implicit rounding in QuantMath.sol div and mul 2. Inappropriate method of casting integer to unsigned integer in SignedConverter.sol...

Can steal collateral deposit

Lines of code Vulnerability details Impact The Collateral.deposit function mints initial shares equal to the deposited amount. The deposit / withdraw functions also use the strategyController.totalValue, which includes the strategy contract balance, to compute the shares. It's possible to increas...

SingleStrategyController doesn't verify that new strategy uses the same base token

Lines of code Vulnerability details Impact When migrating from one strategy to another, the controller pulls out the funds of the old strategy and deposits them into the new one. But, it doesn't verify that both strategies use the same base token. If the new one uses a different base token, it...

Malicious User can Manipulate PreCT Mint Logic with Direct Base Token Transfer

Lines of code Vulnerability details Impact A bad actor can steal funds from future depositors by sending the base token directly to the Strategy or StrategyController contracts. This exploit is more effective the less shares that have already been distributed, perhaps early into the launch of the...

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 2 wei so that it i...

Manipulating PreCT Mint Logic with Direct Base Token Transfer

Lines of code Vulnerability details Impact A bad actor can steal funds from future depositors by sending the base token directly to the Strategy or StrategyController contracts. This exploit is more effective the less shares that have already been distributed, perhaps early into the launch of the...

Reward calculations can be rendered to zero due to the lack of precision

Lines of code Vulnerability details Impact On a combination of high enough token value and low enough decimals there can be not enough precision to store reward amount, which can be permanently hid from a user as a result. I.e. on such a combination there will effectively be no rewards for some...

Too many amms - omik

Duplicate of 97 --- The text was updated successfully, but these errors were encountered: All reactions...

Governance issue - robee

Title: Usage of an incorrect version of Ownbale library can potentially malfunction all onlyOwner functions Severity: High Risk --- The text was updated successfully, but these errors were encountered: All reactions...