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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Future Epoch Pending Redemption Balance Set Vulnerability

Lines of code Vulnerability details Impact The setPendingRedemptionBalance function allows an admin to set the balance of a user's pending redemption for a specific epoch. However, it does not properly check that the epoch being set is in the past and not in the future. An attacker could...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

[Medium-2] Sponsored transactions can be abused for profit

Lines of code Vulnerability details Impact When a user executes a transaction with their SmartAccount, the function execTransaction can be used. In some cases, some protocols want to get promoted and will sponsorize users of the abstract wallet transactions by refunding them back their gas usage ...

validateUserOp does not check if missingAccountFunds matchs the signature

Lines of code Vulnerability details validateUserOp function does not check to see if missingAccountFunds uint256 matchs the signature or not . this can be abused if an attacker front run the tx and call validateUserOp in entrypoint using same sig but with a different missingaccountfunds number...

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

Failed transactions may lead to fund loss via replay attacks

Lines of code Vulnerability details Impact Failed transactions inside executeTransaction will revert and make the nonces remain intact. Thus a replay attack with signature arguments is possible. Proof of Concept One can make replay attacks with the reverted executeTransaction function and reprodu...

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