JumpRateModelV2 may return wrong values

Lines of code Vulnerability details JumpRateModelV2 may return wrong values Impact Solidity integer division might truncate. As a result, performing multiplication before division can sometimes avoid loss of precision. Vulnerability Details In general, this is a problem due to precision mostly if...

Admin should be able to refund or redeem the sanctioned users

Lines of code Vulnerability details Impact Sanctioned user's funds are locked Proof of Concept It is understood that the sanctioned users can not mint nor redeem because the functions requestMint and requestRedemption are protected by the modifier checkKYC. And it is also understood that the...

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

mintFee can be set to 100%

Lines of code Vulnerability details Impact The mintFee can be set to 100%. An amount of centralization over the usual is to be expected in a protocol that deals with RWAs and requires KYC but being able to set the fee to 100% is unnecessary, dangerous, and could harm Ondo's reputation. Proof of...

Admin/owner role lockout possible

Lines of code Vulnerability details Impact In the cash part of the code, the contract TokenProxy in Proxy.sol serves as a generic proxy for all tokens deployed using factory contracts. The factory contracts deploy a ProxyAdmin contract and a TokenProxy, alongside the respective token...

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

Signatures can be misused to reverify

Lines of code Vulnerability details Impact It is possible to remove the KYC status of any user by using the removeKYCAddresses function. This could be easily overridden by user using the addKYCAddressViaSignature function as shown below Proof of Concept 1. User KYC is approved and he is provided...

Unrestricted ComptrollerInterface and InterestRateModel Contract Changes by Admin and PendingAdmin leading to Loss of Funds for Users

Lines of code Vulnerability details Impact // Maximum borrow rate that can ever be applied .0005% / block uint internal constant borrowRateMaxMantissa = 0.0005e16; // Maximum fraction of interest that can be set aside for reserves uint internal constant reserveFactorMaxMantissa = 1e18; The...

Admin account can lose user's collateral

Lines of code Vulnerability details Impact This is high risk because funds can be send to the wrong address. Proof of Concept Tools Used VSCode, Slither Recommended Mitigation Steps Recommend considering the use of msg.sender in completeRedemptions and processRedemption --- The text was updated...

Access control

Lines of code Vulnerability details Impact In contract OndoPriceOracle.sol, there is use of a dependency Ownable.sol for access control. The project should use Openzeppelins implementation of Ownable preferably the from the sub library Openzeppelin/contracts-upgradble. If the dev team argues for...

Anybody can initialize claimMint of others people

Lines of code Vulnerability details Impact this can cause poor management of the tokens on the go, without the end customer being aware that any other person will be able to start the claim process without the consent of the owner of the tokens. This is caused by the fact that a KYC check is made...

Unchecked call to transferOwnership() function in CashFactory contract

Lines of code Vulnerability details Summary The deployCash function in the CashFactory contract calls the transferOwnership function on the cashProxyAdmin contract without checking if the call succeeded or not. If the call to transferOwnership fails, the assertcashProxyAdmin.owner == guardian; wi...

Upgradeable

Lines of code Vulnerability details Impact Since the contract OndoPriceOracle.sol is a critical contract it should have room to be upgrade, either use the library @openzeppelin/contracts-upgradeable or leave a storage gap. An example of this would be to add this line: uint25650 private gap; Proof...

Attacker can spam mint and redeem to DOS others if mintFee = 0

Lines of code Vulnerability details Impact This issue is more about the design of CashManager. In CashManager contract, each epoch has a limit for total mint and redeem amount. Attacker can abused this limit, spam minting, redeeming and repeat to DOS other users. In addition, mintFee is initializ...

Malleable value in S

Lines of code Vulnerability details Impact Signature Proof of Concept ECDSA's malleability can be used to create new signatures under specific conditions. This post explores how serious these attacks can be. Tools Used Recommended Mitigation Steps Recomendation is to be added additional check for...

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

If requestMint() is called before mintFee is initially set via setMintFee(), the return of _getMintFees(collateralAmountIn) can be "underflow" , which results in the loss for the protocol and the unexpected-benefit for certain users

Lines of code Vulnerability details Impact This vulnerability lead to the loss for this protocol and the unexpected-benefit for certain users like this: The protocol cannot earn the fee in collateral feesInCollateral that is supposed to be earned unless the MANAGERADMIN set the certain amount of...

Potential DOS in utilizationRate() function

Lines of code Vulnerability details Potential DOS IN UTILIZATIONRATE utilizationrate = borrows / cash + borrows - reserves. if cash+ borrows = reserves this goes to infinity so that utilizationrate reverts. Also if cash + borrows Tools Used Vs code Recommended Mitigation Steps Define...

lastSetMintExchangeRate is not updated

Lines of code Vulnerability details Impact The lastSetMintExchangeRate variable is not updated if exchange rate deviates the max amount. This becomes a problem since once Admin unpauses the system and new Exchange rate are being set, lastSetMintExchangeRate still points to exchange rate from last...

Use safeTransefer and safeTransferfrom

Lines of code Vulnerability details Impact and Recommended Mitigation Steps CErc20.sol Transfers are not safe because there is no checking whether the return value is true/false without use of Openzeppelins safeTransfer/safeTransferFrom. Should use the...

Loss of user funds when completing CASH redemptions

Lines of code Vulnerability details The function completeRedemptions present in the CashManager contract is used by the manager to complete redemptions requested by users and also to process refunds. function completeRedemptions address calldata redeemers, address calldata refundees, uint256...

Upgraded G -> 3 from #90 [1673938387276]

Judge has assessed an item in Issue 90 as 3 risk. The relevant finding follows: fenkge;gk --- The text was updated successfully, but these errors were encountered: All reactions...

Centralization risk

Lines of code Vulnerability details Impact MANAGERADMIN can set a very high value for minimumRedeemAmount which will ensure that all attempt to redeem amount simply fails Proof of Concept 1. User A wants to redeem amount 100000 using the requestRedemption function 2. Before User A can do so,...

setPendingRedemptionBalance() fails to modify currentRedeemAmount when epoch == currentEpoch

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. setPendingRedemptionBalance fails to modify currentRedeemAmount when epoch == currentEpoch. This is necessary since when epoch == currentEpoch, if redemptionInfoPerEpochepoch.addressToBurnAmtuser is...

Liquidation bots are needed to stabilize the system.

Lines of code Vulnerability details Impact Allowing liquidation bots to arbitrage would stabilize the system. The majority of liquidations are done by liquidation bots. For liquidation bots to be viable there has to exist a secondary market where the assets can be instantly sold. This is especial...

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

Incorrect Authorization in setPendingRedemptionBalance function can lead receiving more collateral

Lines of code Vulnerability details Impact An attacker to exploit the setPendingRedemptionBalance function if they are able to gain the MANAGERADMIN role. In the provided code, the function allows the MANAGERADMIN to set the pending redemption balance of a user for a specific epoch. If an attacke...

Insufficient Input Validation Leading to Excessive Token Burn.

Lines of code Vulnerability details Impact This vulnerability allows a malicious user to redeem an amount of cash tokens that is less than the minimum redemption amount or more than the user's actual balance. This bug can be considered as a high severity bug, as it allows an attacker to burn an...

Rounding error "confiscastes" some collateral

Lines of code Vulnerability details Impact There are no adjustments for collateral that is not returned on redemptions due to rounding errors. It will stay in the assetSender however, it will look as if was fully paid out. The rounding error also depends on the burned quantity, so it may be...

Users have no guarantee of a minimum amount received when minting and redeeming

Lines of code Vulnerability details Impact It is a widely accepted practice in DeFi to offer users the ability to specify the least favorable exchange they are willing to accept. Since CASH tokens are not received in the same transaction it is not possible to create a wrapper that checks if they...

exchangeRateDeltaLimit can be exceeded with overrideExchangeRate() and remain unpaused

Lines of code Vulnerability details Impact The CashManger has a safety feature that limits the maximum change in the exchange rate between epochs. in setMintExchangeRate it is checked that this limit is not exceeded but there is no check in overrideExchangeRate. Proof of Concept An epoch could ha...

CASH token cannot be transferred among users and accounts

Lines of code Vulnerability details Impact Since the CASH token cannot be transferred unless it is explicitly allowed, the users cannot send tokens between accounts, trade it on DEXes, or integrate it into their protocols. This is a non-standard behavior that should be reported to them. Proof of...

The calculation logic for the currentEpochStartTimestamp in the constructor of the CashManager.sol is wrong

Lines of code Vulnerability details Impact This wrong calculation logic for the currentEpochStartTimestamp lead to the situation the currentEpoch that the wrong result is assigned will be stored into the storages in the several functions in the CashManager.sol. Proof of Concept Epoch parameters...

No integrity between completeRedemptions calls for the same epoch

Lines of code Vulnerability details Impact The method completeRedemptions accepts both the list of redeeming accounts that should be processed, and an amount to disburse among them as parameters. As iterating over ALL the redeeming accounts in a given epoch can exceed the block gas limit, it is...

Reuse of signature to get KYCd after it has been removed

Lines of code Vulnerability details Impact There is no time limit on the validity off KYC digests and users with a removed KYC are not saved. If a issuer of such a digest is either compromised or if they by mistake issue a digest with a deadline far into the future a user could reuse the same...

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

Allowance isn't reduced on transfer if it is type(uint).max

Lines of code Vulnerability details Impact Allowance isn't reduced on transfer if it is typeuint.max. By design of the ERC20 token, if the spender is not the sender, allowance must always be deducted after the transfer. Proof of Concept / Get the allowance, infinite for the account owner / uint...

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

Re-use Signature in different chains and contract from the same chain

Lines of code Vulnerability details Impact There is a signature vulnerability where you can re-use it in different chains and contracts. The vulnerability relies in the fact that there is no chainId specified neither addressthis inside the hash expected message: bytes32 expectedMessage =...

Signature Replay no nonce

Lines of code Vulnerability details Impact There is a signature replay vulnerability. That means that the signature can be reused in the same contract from anyone calling. The vulnerability relies in the fact that there is no nonce specified for every caller. This means that you can take a...

Use reentrancyGuard for safeTransferFrom

Lines of code Vulnerability details Impact A reentrancy guard is a mechanism used to prevent reentrant calls in smart contracts, which can be a security vulnerability. Reentrancy can happen when a malicious contract calls the "safeTransferFrom" function and then calls it again before the first ca...

Upgraded G -> 3 from #31 [1673740145531]

Judge has assessed an item in Issue 31 as 3 risk. The relevant finding follows: tetaegerageage --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded G -> 3 from #31 [1673740310963]

Judge has assessed an item in Issue 31 as 3 risk. The relevant finding follows: gegae --- The text was updated successfully, but these errors were encountered: All reactions...

Calling execute() and executeBatch() functions in SmartAccount.sol from the EntryPoint will fail

Lines of code Vulnerability details Impact The function requireFromEntryPointOrOwner is being called within the execute and executeBatch functions to check if the msg.sender is either the owner or the EntryPoint contract, but these functions have onlyOwner modifier, which will only allow the owne...

when the 'v' value from a signature is 0, checkSignatures function can be tricked

Lines of code Vulnerability details Impact when the v value of a signature is = 0, the 'checkSignatures' in entryPoint function doesn't check if the signer is the owner of the wallet and assumes that it is from a contract, a malicious party could craft a signature with the v value = 0, and...

SmartAccount authorization can be bypassed using a contract signature

Lines of code Vulnerability details SmartAccount authorization can be bypassed using a contract signature The SmartAccount wallet supports contract signatures defined by EIP1271, similar to how Gnosis Safe does. Transactions to the wallet can be authorized by a contract that implements the...

Proxy creation isn't check in deployWallet function of SmartAccountFactory contract

Lines of code Vulnerability details The deployWallet function present in the SmartAccountFactory contract deploys a new wallet by creating a Proxy that points to a base implementation using assembly. function deployWalletaddress owner, address entryPoint, address handler public returnsaddress pro...

Relayers can steal extra fees from smart contract wallets on every transaction

Lines of code Vulnerability details Impact Relayers can take signed transactions and append zeroes to the signature parameter to artificially increase the gas cost and startGas estimation. This causes additional cost for the signer and increases the relayers reimbursement. The cost/reimbursement...