Liquidated positions can potential have non-zero debts, making users who deposit to the positions suffer from unfair debt

Handle WatchPug Vulnerability details In the current implementation, new borrows will be charged a 0.5% interest right away. Making the borrower be recorded a 100.5% amount of debt. However, when a position got liquidated, the unrealized interest may still remain in the position while the...

MochiProfileV0.registerAsset() Can Reset Existing Asset Classes

Handle leastwood Vulnerability details Impact registerAsset is used to add assets to the Mochi Protocol. These assets have an associated asset class which represents an asset's risk factor. registerAsset can be called by any user and abused in such a way that existing assets can have their...

Owner has unlimited minting capability

Handle Koustre Vulnerability details Impact Malicious owner is able to abuse the minting capability of the token USDM. Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. Tools Used Recommended...

anyone can create a vault by directly calling the factory

Handle jonah1005 Vulnerability details Impact MochiVaultFactory.solL26-L37 There's no permission control in the vaultFactory. Anyone can create a vault. The transaction would be reverted when the government tries to deploy such an asset. As the protocol checks whether the vault is a valid vault b...

Improper Validation Of create2 Return Value

Handle leastwood Vulnerability details Impact The BeaconProxyDeployer.deploy function is used to deploy lightweight proxy contracts that act as each asset's vault. The function does not revert properly if there is a failed contract deployment or revert from the create2 opcode as it does not...

liquidation factor < collateral factor for Sigma type

Handle cmichel Vulnerability details The MochiProfileV0 defines liquidation and collateral factors for different asset types. For the AssetClass.Sigma type, the liquidation factor is less than the collateral factor: function liquidationFactoraddress asset public view override returns float memory...

Early small positions may not be able to be liquidated

Handle WatchPug Vulnerability details In the current implementation, new borrows will be charged a 0.5% interest right away. Making the borrower be recorded a 100.5% amount as debt. However, the pre-charged 0.5% interest is not included in the vault's debts. As a result, early small positions in ...

treasuryShare is Overwritten in FeePoolV0._shareMochi()

Handle leastwood Vulnerability details Impact The FeePoolV0.sol contract accrues fees upon the liquidation of undercollaterised positions. These fees are split between treasury and vMochi contracts. However, when distributeMochi is called to distribute mochi tokens to veCRV holders, both mochiSha...

Unhandled return values of transfer

Handle WatchPug Vulnerability details ERC20 implementations are not always consistent. Some implementations of transfer could return ‘false’ on failure instead of reverting. It is safer to wrap such calls into require statements to these failures. Unsafe transfer calls were found in the following...

UniswapV2/SushiwapLPAdapter update the wrong token

Handle cmichel Vulnerability details The UniswapV2LPAdapter/SushiswapV2LPAdapter.update function retrieves the underlying from the LP token pair asset but then calls router.updateasset, proof which is the LP token itself again. This will end up with the router calling this function again...

Lack of data validation in update function

Handle Koustre Vulnerability details Impact There is no data validation of the data input into update function in MochiCSSRv0.sol. Allowing anyone to set the price of any non ERC20 token that is not a bluechip asset DAI, WETH, ETH, etc. Proof of Concept Provide direct links to all referenced code...

Chainlink's latestRoundData might return stale or incorrect results

Handle nikitastupin Vulnerability details Proof of Concept The ChainlinkAdapter calls out to a Chainlink oracle receiving the latestRoundData. If there is a problem with Chainlink starting a new round and finding consensus on the new value for the oracle e.g. Chainlink nodes abandon the oracle,...

Possible to restart liquidation process after liquidation is settled

Handle harleythedog Vulnerability details Impact In the triggerLiquidation function, there is a require statement as follows: requireauction.startedAt == 0 || auction.boughtAt != 0, "on going"; However, after an auction has been settled, we have auction.boughtAt != 0, so this require statement wi...

DutchAuctionLiquidator.settleLiquidation() Does Not Operate On Non-Standard ERC20 Tokens

Handle leastwood Vulnerability details Impact settleLiquidation does not validate the return value of an ERC20 transfer call. As a result, if the call to transfer does not revert but instead returns a boolean or void value, it may be possible that a user liquidates a user's position but does not...

Chainlink price data could be stale

Handle WatchPug Vulnerability details function getPriceaddress asset public view override returns float memory , int256 price, , , = feedasset.latestRoundData; uint256 decimalSum = feedasset.decimals + IERC20Metadataasset.decimals; if decimalSum 18 return float numerator: uint256price, denominato...

MochiCSSRv0.update() Does Not Operate Correctly On Bluechip Assets

Handle leastwood Vulnerability details Impact Mochi vaults query price feeds for updates via the update function in MochiCSSRv0.sol. If the asset to be queried is a bluechip asset, the function will call getPrice on an adapter which adheres to the ICSSRAdapter interface. If the adapter is not...

ChainLink price data could be stale

Handle cmichel Vulnerability details There is no check in ChainlinkAdapterEth.getPrice if the return values indicate stale data. This could lead to stale prices according to the Chainlink documentation: under current notifications: "if answeredInRound roundId could indicate stale data." under...

Changing engine.nft contract breaks vaults

Handle cmichel Vulnerability details Governance can change the engine.nft address which is used by vaults to represent collateralized debt positions CDP. When minting a vault using MochiVault.mint the address returned ID will be used and overwrite the state of an existing debt position and set it...

UniswapV2TokenAdapter does not support Sushiswap-only assets

Handle cmichel Vulnerability details The UniswapV2TokenAdapter.supports function calls its aboveLiquidity function which returns the UniswapV2 liquidity if the pair exists. If this is below minimumLiquidity, the supports function will return false. However, it could be that the Sushiswap pair has...

Debt accrual is path-dependant and inaccurate

Handle cmichel Vulnerability details The total debt in MochiVault.accrueDebt increases by the current debt times the debt index growth. This is correct but the total debt is then reduced again by the calling user's discounted debt, meaning, the total debt depends on which specific user performs t...

A malicious user can potentially escape liquidation by creating a dust amount position and trigger the liquidation by themself

Handle WatchPug Vulnerability details In the current implementation, a liquidated position can be used for depositing and borrowing again. However, if there is a liquidation auction ongoing, even if the position is now liquidatable, the call of triggerLiquidation will still fail. The liquidator...

ERC20 return values not checked

Handle cmichel Vulnerability details The ERC20.transfer and ERC20.transferFrom functions return a boolean value indicating success. This parameter should checked for success. See VestedRewardPool.claim which performs ERC20 transfers without checking for the return value. Impact As the trusted moc...

Historic data being requested as a part of MochiVault.withdraw and borrow functions can be outdated, so a user can avoid historic data update with sending old piece of _data

Handle hyh Vulnerability details Impact Asking to provide historic data proof doesn't imply that pricing is current, a malicious user can wait for market volatility and do deposit/borrow sequence with outdated price, borrowing more than current market value of supplied assets for example, suppose...

Chainlink Adapter Missing Validation Of latestRoundData() Outputs

Handle leastwood Vulnerability details Impact ChainlinkAdapter.getPrice queries a Chainlink oracle to retrieve the latest price for a given asset. However, this external call does not validate the data retrieved is fresh. Proof of Concept Tools Used Manual code review Recommended Mitigation Steps...

[DutchAuctionLiquidator.sol] The return value of an external transfer call is not checked

Handle nikitastupin Vulnerability details Impact Some tokens do not revert in case of failure and return false instead. If one of these tokens is used in Mochi, settleLiquidation will not revert if the transfer fails, and an attacker can call settleLiquidation for free. Proof of Concept Tools Use...

Avoid Using Block.timestamp in checkClaimable

Handle Koustre Vulnerability details Impact checkClaimable function is dependent on using block.timestamp and this can be manipulated to claim funds earlier than it should in functions claim, forceClaim and lock Proof of Concept Provide direct links to all referenced code in GitHub. Add...

Vault fails to track debt correctly that leads to bad debt

Handle jonah1005 Vulnerability details Impact It's similar to the issue "misuse amount as increasing debt in the vault contract". Similar issue in a different place that leads to different exploit patterns and severity. When users borrow usdm from a vault, the debt increased by the amount 1.005...

One auction at a time would lead to bad debt

Handle jonah1005 Vulnerability details One auction at a time would lead to bad debt Impact The current DutchAuctionLiquidator only allows one auction for a position at a time. This is not a desirable design. If a position is liquidated due to the price movement of collaterals, the position would...

Liquidation will never work with non-zero discounts

Handle harleythedog Vulnerability details Impact Right now, there is only one discount profile in the github repo: the "NoDiscountProfile" which does not discount the debt at all. This specific discount profile works correctly, but I claim that any other discount profile will result in liquidatio...

Anyone can extend withdraw wait period by depositing zero collateral

Handle harleythedog Vulnerability details Impact In MochiVault.sol, the deposit function allows anyone to deposit collateral into any position. A malicious user can call this function with amount = 0, which would reset the amount of time the owner has to wait before they can withdraw their...

Unchecked ERC20 transfer calls

Handle loop Vulnerability details ERC20 transfer and transferFrom calls normally return true on a succesful transfer. In DutchAuctionLiquidator the call asset.transfermsg.sender, collateral; is made. asset refers to whichever ERC20 asset is used for the vault of that auction. If asset is an ERC20...

feePool is vulnerable to sandwich attack.

Handle jonah1005 Vulnerability details Impact There's a permissionless function distributeMochi in FeePoolV0. Since everyone can trigger this function, an attacker can launch a sandwich attack with flashloan to steal the funds. FeePoolV0.solL55-L62 The devs have mentioned this concern in the...

Changing NFT contract in the MochiEngine would break the protocol

Handle jonah1005 Vulnerability details Impact MochiEngine allows the operator to change the NFT contract. MochiEngine.solL91-L93 All the vaults would point to a different NFT address. As a result, users would not be access their positions. The entire protocol would be broken. IMHO, A function tha...

treasury is vulnerable to sandwich attack

Handle jonah1005 Vulnerability details treasury is vulnerable to sandwich attack. Impact There's a permissionless function veCRVlock in MochiTreasury. Since everyone can trigger this function, the attacker can launch a sandwich attack with flashloan to steal the funds. MochiTreasuryV0.solL73-L94...

Referrer can drain ReferralFeePoolV0

Handle gzeon Vulnerability details Impact function claimRewardAsMochi in ReferralFeePoolV0.sol did not reduce user reward balance, allowing referrer to claim the same reward repeatedly and thus draining the fee pool. Proof of Concept L28-47 did not reduce user reward balance Tools Used None...

registerAsset would change asset class of existing asset

Handle jonah1005 Vulnerability details Impact The permissionless function registerAsset only checks current liquidity. There's no sanity check whether the asset has registered as another asset class. An attacker can set an asset into AssetClass.Sigma. Unexpected liquidation would happen if an Alp...

registerAsset() can overwrite _assetClass value

Handle gpersoon Vulnerability details Impact Everyone can call the function registerAsset of MochiProfileV0.sol Assuming the liquidity for the asset is sufficient, registerAsset will reset the assetClass of an already registered asset to AssetClass.Sigma. When the assetClass is changed to...

debts calculation is not accurate

Handle gpersoon Vulnerability details Impact The value of the global variable debts in the contract MochiVault.sol is calculated in an inconsistent way. In the function borrow the variable debts is increased with a value excluding the fee. However in repay and liquidate it is decreased with the...

Unused ERC20 tokens are not refunded

Handle WatchPug Vulnerability details Based on the context and comments in the code, we assume that it's possible that there will be some leftover sell tokens, not only when users are selling unwrapped ETH but also for ERC20 tokens. However, in the current implementation, only refunded ETH is...

Change require conditions can prevent fund loss when called with mistaken input data

Handle WatchPug Vulnerability details function swapByQuote address zrxSellTokenAddress, uint256 amountToSell, address zrxBuyTokenAddress, uint256 minimumAmountReceived, address zrxAllowanceTarget, address payable zrxTo, bytes calldata zrxData, uint256 deadline external payable whenNotPaused...

use of transfer() instead of call() to send eth

Handle JMukesh Vulnerability details Impact Use of transfer might render ETH impossible to withdraw becuase after istanbul hardfork , there is increases in the gas cost of the SLOAD operation and therefore breaks some existing smart contracts.Those contracts will break because their fallback...

Swap.sol implements potentially dangerous transfer

Handle elprofesor Vulnerability details Impact The use of .transfer in Swap.sol may have unintended outcomes on the eth being sent to the receiver. Eth may be irretrievable or undelivered if the msg.sender or feeRecipient is a smart contract. Funds can potentially be lost if; 1. The smart contrac...

transfer() is not recommended for sending ETH

Handle WatchPug Vulnerability details Since the introduction of transfer, it has typically been recommended by the security community because it helps guard against reentrancy attacks. This guidance made sense under the assumption that gas costs wouldn’t change. It's now recommended that transfer...

Wrong calculation of erc20Delta and ethDelta

Handle WatchPug Vulnerability details function fillZrxQuote IERC20 zrxBuyTokenAddress, address payable zrxTo, bytes calldata zrxData, uint256 ethAmount internal returns uint256, uint256 uint256 originalERC20Balance = 0; if!signifiesETHOrZeroaddresszrxBuyTokenAddress originalERC20Balance =...

fillZrxQuote doesn't return correct values when zrxSellTokenAddress == zrxBuyTokenAddress

Handle harleythedog Vulnerability details Impact Suppose that swapByQuote is called with zrxSellTokenAddress == zrxBuyTokenAddress, and neither of these addresses "signifiesETHOrZero". The contract first transfers amountToSell of these tokens from the sender's account into the contract and update...

Users can avoid paying fees for ETH swaps

Handle pants Vulnerability details Users can call Swap.swapByQuote to execute an ETH swap where they receive ETH without paying swap fee for the gained ETH. They can trick the system by setting zrxBuyTokenAddress to an address of a malicious contract and making it think they have executed an ERC2...

Arbitrary contract call allows attackers to steal ERC20 from users' wallets

Handle WatchPug Vulnerability details function fillZrxQuote IERC20 zrxBuyTokenAddress, address payable zrxTo, bytes calldata zrxData, uint256 ethAmount internal returns uint256, uint256 uint256 originalERC20Balance = 0; if!signifiesETHOrZeroaddresszrxBuyTokenAddress originalERC20Balance =...

.transfer is used for transferring ether

Handle pauliax Vulnerability details Impact payablemsg.sender.transfertoTransfer; feeRecipient.transferaddressthis.balance; It is no longer recommended to use .transfer when sending ether as recipients with custom fallback functions smart contracts will not be able to handle that. You can read mo...

Usage of transfer

Handle cmichel Vulnerability details The address.transfer function is used in sweepFees and and swapByQuote to send ETH to an account. It is restricted to a low amount of GAS and might fail if GAS costs change in the future or if feeRecipient is a smart contract and its fallback function handler...

Transfer function is unreliable

Handle Koustre Vulnerability details Impact Opcode pricing is not stable and should not be relied upon to protect against re-entrancy attacks. Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept...