managementFee is unfair and can be used to steal stakers deposits

Lines of code Vulnerability details Description managementFee is a fee that is taken on TVL and calculated per year: File: Vault.sol 429: function accruedManagementFee public view returns uint256 430: uint256 managementFee = fees.management; 431: return 432: managementFee 0 433: ?...

Upgraded Q -> 2 from #748 [1675439377792]

Judge has assessed an item in Issue 748 as 2 risk. The relevant finding follows: L MultisigManager doesn't have unregister function MultisigManager.sol contains a hard limit of MULTISIGLIMIT to 10. Meanwhile in the contract, there are functions like registerMultisig, enableMultisig, and...

cidFeeWallet can be address(0)

Lines of code Vulnerability details Impact TLDR; the cidFeeWallet can be assigned to address0, in which case fees would be burned but otherwise the system operates as expected. In the constructor for both SubprotocolRegistry and CidNFT, a wallet is assigned to which fees will be sent. This applie...

Unchecked that address to is not 0

Lines of code Vulnerability details Impact Sanity checks checking that funds are not going to the burn address should be done. In this case in the function swap, funds will not burn because you have to return the flashloan but the check should be made Tools Used manual Recommended Mitigation Step...

mint() function logic will break with fee-on-transfer(deflationary) tokens

Lines of code Vulnerability details Impact with deflationary token mint function never succeed Proof of Concept mint function checking if balanceAfter totalLiquidity revert CompleteUtilizationError; // next check is for the case when liquidity is borrowed but then was completely accrued if...

Mitigation of H-05: Issue not mitigated

Lines of code Vulnerability details Mitigation of H-05: Issue not mitigated --- The text was updated successfully, but these errors were encountered: All reactions...

Gas Optimizations

See the markdown file with the details of this report here. --- The text was updated successfully, but these errors were encountered: All reactions...

All Transfer Hooks have incomplete checks which might lead to transactions reverting

Lines of code Vulnerability details Impact Due to not checking that the ids array and amounts array are same , if given the two arraya of non-equal length it might lead to incorrect token transfers and unwanted transaction reverts. Proof of Concept According to the EIP specification the ids array...

Allows burning from other accounts

Lines of code Vulnerability details Impact Anyone can burn the liquidamount before maturity by calling burn in Pool.sol Proof of Concept 364 : liquidityPosition.mintliquidityAmount; liquidityAmount is minted by liquidityPosition but when burned, it is not clear who will burn liquidityAmount 452: ...

ATTACKER CAN STEAL FUNDS BECAUSE OF REENTRANCY VULNERABILITY IN burn METHOD

Lines of code Vulnerability details Impact In burn method of TimeswapV2Option contract, there is no Reentrancy guard. The method allows a Callback function before updating the State variable, which makes method vulnerable for Reentrancy which can lead to steal of funds. Proof of Concept File:...

Upgraded Q -> M from #598 [1674741121008]

Judge has assessed an item in Issue 598 as M risk. The relevant finding follows: Low: 1.deposit check wrong variant function deposituint256 assets, address receiver public virtual returns uint256 shares requireshares minDepositAmount, "VALUETOOSMALL"; requireassets minDepositAmount,...

Upgraded Q -> M from #472 [1674665995647]

Judge has assessed an item in Issue 472 as M risk. The relevant finding follows: L-01 CHAINLINK AGGREGATOR IS NOT SUFFICIENTLY VALIDATED AND CAN RETURN STALE ANSWER As shown below, calling the getAssetPrice function in the ParaSpaceOracle contract can execute price = uint256source.latestAnswer,...

Adversary can abuse a quirk of compound redemption to manipulate the underlying exchange rate and maliciously disable cToken collaterals

Lines of code Vulnerability details Impact Adversary can maliciously disable cToken collateral to cause loss to rToken during restructuring Proof of Concept if referencePrice 0: / if redeemTokensIn 0 / We calculate the exchange rate and the amount of underlying to be redeemed: redeemTokens =...

function withdraw() in StRSR won't update contract state (totalDrafts) in all cases which can cause wrong fund distribution and fund stucking in the contract

Lines of code Vulnerability details Impact Function withdraw complete an account's unstaking. it transfers user draft withdrawals and updates totalDrafts. but when calculated rsrAmount is 0 code returns and won't updates totalDrafts which can cause wrong calculations as those draft items removed...

High Severity Reentrancy Vulnerability in stateTransition Modifier

Lines of code Vulnerability details Impact Lack of reentrancy protection in the stateTransition modifier occurs in the following line of code. modifier stateTransitionTradeStatus begin, TradeStatus end requirestatus == begin, "Invalid trade state"; status = TradeStatus.PENDING; ; assertstatus ==...

Reentrancy Vulnerability in stake() and unstake() functions.

Lines of code Vulnerability details Impact An attacker can repeatedly call these functions and manipulate the state of the contract, potentially leading to a loss of funds for the users of the contract. For example, an attacker could repeatedly call the stake function, causing the attacker to...

WithdrawProxy will break if there is a fee-on-transfer underlying

Lines of code Vulnerability details PublicVault.transferWithdrawReserve will increase WithdrawProxy.withdrawReserveReceived by withdrawBalance. The issue is that if the token has a fee-on-transfer, withdrawBalance will be greater than the amount received by withdrawProxy 384:...

Redeemers will receive less collateral than expected if function completeRedemptions() is called with empty refundees list

Lines of code Vulnerability details Impact Function completeRedemptions is used by admin account to distribute collateral to users and also to refund redemption requests if the redemption cannot be serviced. function completeRedemptions address calldata redeemers, address calldata refundees,...

Deny of service in CCash.sol with "transfer" which can be unusable for smart contract calls

Lines of code Vulnerability details Impact The CCash.doTransferOut method is susceptible to denial of service. Proof of Concept The logic of the doTransferOut method in CCash is as follows: The whole user withdraw is being handled with a token.transfer call. This is unsafe as transfer has hard...

Upgradable liberty

Lines of code Vulnerability details Impact Instead of using @openzeppelin/contracts, use the upgradable liberty for contracts that should be able to be upgraded. This is the library that should be used :@openzeppelin/contracts-upgradeable Proof of Concept For more info have a look at this resourc...

Not resetting totalBurned in CashManger will break user redemptions

Lines of code Vulnerability details Not resetting totalBurned in CashManger will break user redemptions The current implementation in CashManager.completeRedemptions is not updating the totalBurned amount in an epoch if there was a refund. The problem is, that if not all user redemptions can be...

_processRefund() fails to update currentRedeemAmount when epochToService == currentEpoch

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. processRefund fails to update currentRedeemAmountwhenepochToService == currentEpoch. As a result, currentRedeemAmount will contain the portion that has already been refunded, an incorrect number. Proof ...

MANAGER_ADMIN has the right to give or take any amount of user's tokens using setPendingMintBalance.

Lines of code Vulnerability details Impact Not sure what the use case of setPendingMintBalance should be, but it can cause admins to drain the whole balance, nullify the user's funds or break the protocol. This can be caused on purpose, accidentally or through an attacker stealing MANAGERADMIN's...

Values in redemptions do not match, and fees are being overcharged

Lines of code Vulnerability details Impact The amount of collateral to distribute collateralAmountToDist does not need to match the sum of fees actually distributed the sum of collateralAmountDue in the events of processRedemption. The real distributed amount can be smaller. However, the fees are...

If completeRedemptions is called multiple times to redeem one epoch, the complex calculation may result in incorrect redemptions

Lines of code Vulnerability details Impact When MANAGERADMIN calls the completeRedemptions function, it requires that both redeemers and refundees have KYC. If the redeemer loses KYC, the redeemer's address will not appear in redeemers and refundees, otherwise completeRedemptions will fail...

Anyone can be refunded; no input validation

Lines of code Vulnerability details Impact The method for processing refunds has no input validation. If Bob burns 100 CASH and Alice burns 1 CASH, and the manager wants to refund Alice and pay out Bob, they are able to swap the refund parameters. This would result in Alice getting the entire...

KYC signature can be reused to regain KYC status

Lines of code Vulnerability details The function addKYCAddressViaSignature of the KYCRegistry contract allows a user to be granted a KYC status using a signature provided by Ondo. The function validates that the signer has the corresponding role for the requirement group and adds the user to the...

Wrong logic totalBurned is not updated after _processRefund() results in loss of funds for redeemers

Lines of code Vulnerability details Impact Function completeRedemptions is used by admin account to distribute collateral to users and also to refund redemption requests if the redemption cannot be serviced. function completeRedemptions address calldata redeemers, address calldata refundees,...

Replay attack on different batchId

Lines of code Vulnerability details Description The execTransaction function includes an input parameter called batchId that is used to determine the nonce which is included in the data signed by the owner. However, batchId is not part of the signed data. This allows any third party to replay a...

handleAggregatedOps() does not handle non-atomic transactions which results in whole function revert if one transaction does not go through

Lines of code Vulnerability details Impact Function reverts if one account or paymaster is not validated, which leads to a waste of time and gas. Proof of Concept EntryPoint.UserOpsPerAggregator takes in an array of opsPerAggregator in its parameter and loops through each struct. In the function,...

Users can accidentally lock their stakes forever

Lines of code Vulnerability details Impact StakeManager accepts user deposits and stakes. When adding a new stake, it allows the arbitrary value of unstakeDelaySec supposedly it is higher than previous info.unstakeDelaySec: function addStakeuint32 unstakeDelaySec public payable DepositInfo storag...

Upgraded Q -> M from #216 [1673172689834]

Judge has assessed an item in Issue 216 as M risk. The relevant finding follows: Last collateral check is not safe --- The text was updated successfully, but these errors were encountered: All reactions...

Stealing fund by applying reentrancy attack on removeCollateral, startLiquidationAuction, and purchaseLiquidationAuctionNFT

Lines of code Vulnerability details Impact By applying reentrancy attack involving the functions removeCollateral, startLiquidationAuction, and purchaseLiquidationAuctionNFT, an Attacker can steal large amount of fund. Proof of Concept Bob a malicious user deploys a contract to apply the attack...

Upgraded Q -> M from #27 [1671522165760]

Judge has assessed an item in Issue 27 as M risk. The relevant finding follows: Dup of 45. The effect is severe enough IMO to warrant a med severity although I initially intepreted it as a spec mismatch. --- The text was updated successfully, but these errors were encountered: All reactions...

Stealing liquidity provider's fund by manipulating the reserve

Lines of code Vulnerability details Impact An attacker can steal liquidity provider's fund by manipulating the reserve. During adding a liquidity, the amount of lpToken to be minted will be calculated in the function addQuote. function addQuoteuint256 baseTokenAmount, uint256 fractionalTokenAmoun...

_transferFrom() in Pair contract doesn't update approval amounts when transferring user fractional tokens

Lines of code Vulnerability details Impact Contract Pair is and ERC20 token which represents user fractional token balance and it has all the features of the standard ERC20 tokens. function transferFrom has been written for transferring token but the logic isn't complete and it doesn't consider...

GroupBuy can be drained of all ETH.

Lines of code Vulnerability details Description purchase in GroupBuy faciilitates the purchasing of an NFT after enough contributions were gathered. Another report titled "Attacker can steal the amount collected so far in the GroupBuy for NFT purchase" describes a high impact bug in purchase. It ...

User will often overpay when adding liquidity

Lines of code Vulnerability details Impact One of the assets either baseTokens or fractionalTokens will likely be overpaid when the user calls Pair.add liquidity. Proof of Concept The Pair.add function takes baseTokenAmount, fractionalTokenAmount and minLpTokenAmount as inputs. The first two...

Upgraded Q -> M from #315 [1671270502271]

Judge has assessed an item in Issue 315 as M risk. The relevant finding follows: Lines of code Vulnerability details Description Collateral.sol exposes a permissioned withdraw function: function managerWithdrawuint256 amount external override onlyRoleMANAGERWITHDRAWROLE nonReentrant if...

Lack of validation on price feeds

Lines of code Vulnerability details Impact Lack of validation on Chainlink price feeds may result in incorrectly functioning or non-functioning protocol. For example: during high volatility a price feed may be suspended or become stale; on L2 networks the sequencer might be down on L2 networks th...

winnerClaimNFT() does not check if recipient can receive ERC721

Lines of code Vulnerability details Impact In src\VRFNFTRandomDraw.sol winnerClaimNFT, you should use safeTransferFrom to protect against the case that the recipient cannot receive the ERC721 tokens, and thus will lose the tokens. This is more-so the case when we expect that normal users will be...

Minter privilege escalation vulnerability

Lines of code Vulnerability details Impact The contract owner could potentially abuse their privileges to manipulate the token supply and undermine the integrity of the token economy. Proof of Concept The 'setMinter' function in the contract allows the contract owner to set the minter status for...

Choosing a cheap gas lane may result in no winners

Lines of code Vulnerability details Impact The drawer can choose whichever gas lanekeyHash they like. Giving this choice to the drawer may result in no winners if the network is congested and the drawer chooses a cheap gas lane. Recommended Mitigation Steps Check the options of keyHashes that can...

owner can withdraw the NFT at any time if they wait with starting the draw until after recoverTimelock

Lines of code Vulnerability details Description When creating a random draw the owner specifices a recoverTimelock which is a last resort option to recover the raffled NFT if the draw fails. There are some validations that this is between a week and a year in the future but there's no guarantee...

Wrong address input in BondNFT.createLock results in wrong _owner being saved in memory

Lines of code Vulnerability details Impact Wrong address input in createLock results in function failure of extendLock and claim. Proof of Concept When someone calls lock in Lock.sol, the function BondNFT.createLock is called . bondNFT.createLock asset, amount, period, msg.sender;...

Attacker can disable contract functionality

Lines of code Vulnerability details Impact Current setup of the protocol is vulnerable to a DoS attack. This can be achieved by anyone calling initialize on the implementation VRFNFTRandomDraw contract. With the implementation contract initialized the created clones cannot be re-initialized and...

Wrong update of stoploss in TradingExtension._limitClose()

Lines of code Vulnerability details Impact TradingExtension.limitClose returns a wrong stoploss which is favorable for users and it would be a significant loss for the protocol. Proof of Concept TradingExtension.limitClose is used to set takeprofit/stoploss prices for the pending order and execut...

Chainlink price feed is not sufficiently validated and can return stale price

Lines of code Vulnerability details Impact As mentioned by , "Prices provided by the oracle network are also compared to Chainlink's public price feeds for additional security. If prices have more than a 2% difference the transaction is reverted." The Chainlink price verification logic in the...

Extending period in Lock contract will block user funds

Lines of code Vulnerability details Impact Extending lock will block users' funds. Contract: Lock.sol Proof of Concept Locking tokens with function lock... will update mapping mapping totalLocked for given asset with value of amount, here: totalLockedasset += amount. However, when extending lock...

The owner can swap the proxy implementation with a malicious one

Lines of code Vulnerability details The owner of VRFNFTRandomDrawFactory.sol could swap the current implementation with a malicious one at any moment, without a waiting period. Impact The worse case scenario is one in which the private key of the contract owner gets stolen. In this case the owner...