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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Unbounded Chainlink oracle time delay vulnerability

Lines of code Vulnerability details Summary The contract OndoPriceOracleV2 allows for the owner to set an association between an fToken and a Chainlink oracle for price retrieval. The contract also allows the owner to set a maxmum amount of time delay that it will tolerate from all Chainlink...

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

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

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

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

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

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

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

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

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

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

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

Lines of code Vulnerability details Impact / @notice Contract which oversees inter-cToken operations / ComptrollerInterface public comptroller; / @notice Model which tells what the current interest rate should be / InterestRateModel public interestRateModel; / @notice Administrator for this...

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

Redeemers can reject receiving air-drop collateral token, make function completeRedemptions() failing and wasting gas of admin

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. Redeemers will received funds from assetSender in air-drop manner, if collateral token is...

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

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

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

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

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

setMintLimit and setRedeemLimit can result in DoS

Lines of code Vulnerability details Impact Due to the missing validation of mintLimit and redeemLimit this might result in unreasonably low limits, leading to a DoS of the requestMint and requestRedemption function. One user can simply call these functions with the upper limit, resulting in a DoS...

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

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

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

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

The owner's address is not declared as payable

Lines of code Vulnerability details Impact If someone sends ether to the contract, it would be highly possible that the ether could be locked forever or never withdrawn by its intended recipientOwner Proof of Concept Tools Used Manual Recommended Mitigation Steps Add "payable" keyword to the owne...

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

Front-running of the relayers transaction

Lines of code Vulnerability details Description While the relayer has a positive outcome from the execution of some of the user operations through gas refund it is profitable for the attacker to front-run such transactions with a full copy of those. In such a case attacker receives a positive...

[Medium-3] Non-compliance with EIP-4337

Lines of code Vulnerability details Impact Some parts of the codebase are not compliant with the EIP-4337 from the EIP-4337 specifications, at multiple degrees of severity. Proof of Concept Sender existence Create the account if it does not yet exist, using the initcode provided in the...

SmartAccountFactory.sol - Account can be created for an owner and setting any entryPoint

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. Proof of Concept The entrypoint is being set in the initargs... function of SmartAccount.sol. The problem is that the malicious users could create wallets for legitimate owners of wallets and set the...