FeePoolV0.sol#distributeMochi() will unexpectedly flush treasuryShare, causing the protocol fee cannot be properly accounted for and collected

Handle WatchPug Vulnerability details distributeMochi will call buyMochi to convert mochiShare to Mochi token and call shareMochi to send Mochi to vMochi Vault and veCRV Holders. It wont touch the treasuryShare. However, in the current implementation, treasuryShare will be reset to 0. This is...

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

Missing slippage checks

Handle cmichel Vulnerability details The contracts are missing slippage checks which can lead to being vulnerable to sandwich attacks. A common attack in DeFi is the sandwich attack. Upon observing a trade of asset X for asset Y, an attacker frontruns the victim trade by also buying asset Y, lets...

MochiTreasuryV0.withdrawLock() Is Callable When Locking Has Been Toggled

Handle leastwood Vulnerability details Impact withdrawLock does not prevent users from calling this function when locking has been toggled. As a result, withdraws may be made unexpectedly. Proof of Concept Tools Used Manual code review Recommended Mitigation Steps Consider adding requirelockCrv,...

ReferralFeePoolV0.sol#claimRewardAsMochi() Array out of bound exception

Handle WatchPug Vulnerability details function claimRewardAsMochi external IUSDM usdm = engine.usdm; address memory path = new address; path0 = addressusdm; path1 = uniswapRouter.WETH; path2 = addressengine.mochi; usdm.approveaddressuniswapRouter, rewardmsg.sender; // we are going to ingore the...

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

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

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

Uniswap / Sushiswap prices can be manipulated through flashloans

Handle cmichel Vulnerability details The UniswapV2CSSR.getExchangeRatio uses the current reserve to derive the exchange ratio. The fact that it mixes in historic data does not matter because it still uses the current reserves which can be manipulated through flashloans in currentPriceCumulative...

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

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

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

claimRewardAsMochi will produce a runtime error

Handle pauliax Vulnerability details Impact function claimRewardAsMochi in ReferralFeePoolV0 will produce a runtime exception because the length of the path is 2 but it tries to assign 3 entries: address memory path = new address; path0 = addressusdm; path1 = uniswapRouter.WETH; path2 =...

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

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

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

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

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

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

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

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

MochiVault.sol Potential griefing attack by depositing 0 to target's position

Handle WatchPug Vulnerability details MochiVault.solwithdraw is using the wait modifier to prevent withdraw within delay duration from lastDeposit. However, MochiVault.soldeposit allows anyone to deposit to a specific position. This enables the attacker to initiate a griefing attack by depositing...

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

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

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

Tokens Can Be Stolen By Frontrunning VestedRewardPool.vest() and VestedRewardPool.lock()

Handle leastwood Vulnerability details Impact The VestedRewardPool.sol contract is a public facing contract aimed at vesting tokens for a minimum of 90 days before allowing the recipient to withdraw their mochi. The vest function does not utilise safeTransferFrom to ensure that vested tokens are...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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