Calculation in calculateNewProfit function is broken when jackpot is not won

Lines of code Vulnerability details The function calculateNewProfit present in the LotteryMath library is used when finalizing the current draw in the Lottery to track and update the currentNetProfit variable in the contract. function calculateNewProfit int256 oldProfit, uint256 ticketsSold,...

The claimWinningTickets() function does not include a check to ensure that the caller owns the tickets, or if has already been claimed

Lines of code Vulnerability details The claimWinningTickets function in the contract contains multiple vulnerabilities that can result in a loss of funds for the contract and its users. Firstly, the function can be called by anyone, even if they are not the owner of the ticket, allowing malicious...

Lottery owner can rig the draw to win the jackpot by swapping the source

Lines of code Vulnerability details Lottery owner can rig the draw to win the jackpot by swapping the source Impact The lottery owner has the ability to swap the Random Source under certain cirumstances, and this can be exploited to set a new source contract that returns any number set by it. Thi...

In the beginning its relatively easy to gain majority share

Lines of code Vulnerability details Impact As mentioned in the document, Although possible, the permissions to execute arbitrary actions or upgrade the DAO should not be given to EOAs as this poses a security risk to the organization if the account is compromised or acts adversarial. Instead, the...

execute function should be payable

Lines of code Vulnerability details Impact execute will revert when msg.value 0 Proof of Concept Lacking payable mutability specifier. function execute bytes32 callId, Action calldata actions, uint256 allowFailureMap external override authEXECUTEPERMISSIONID returns bytes memory execResults,...

Funds can be stuck in DAO contract

Lines of code Vulnerability details Impact The DAO contract allows for depositing tokens and native coins, but it does not properly account received funds neither allows it to withdraw them afterwards. Due to that, anyone depositing into DAO contract won't be able to withdraw their funds. Proof o...

setWithdrawalQueue never removes items from the queue and can get out of gas

Lines of code Vulnerability details Impact setWithdrawalQueue calls delete, this sets to 0 each element of the array rather than removing elements what it's done with .pop. After that, strategies are pushed in a for loop, therefore, each time setWithdrawalQueue is called, length of the queue is...

ActivePool.sol – Re-entrancy risk on _rebalance function

Lines of code Vulnerability details The rebalance function is vulnerable to a reentrancy attack. Specifically, an external callee can take over the control flow of the function by calling back into the ActivePool contract via a method that triggers the rebalance function again before it completes...

StabilityPool.sol – Re-entrancy risk on withdrawfromSP()

Lines of code Vulnerability details The withdrawFromSP function in the StabilityPool contract is vulnerable to a reentrancy attack through the sendLUSDToDepositor function. The sendLUSDToDepositor function is vulnerable because it transfers LUSD tokens and LQTY gains before updating state...

No check for active sequencer

Lines of code Vulnerability details Impact The protocol will operate at the previous stale rates. Proof of Concept Chainlink recommends that all Optimistic L2 oracles consult the Sequencer Uptime Feed to ensure that the sequencer is live before trusting the data returned by the oracle, even if...

Contract not initialized after deployment

Lines of code Vulnerability details Impact In ReaperStrategyGranarySupplyOnly.sol, the initialize function is not called after deployment. Left open to unintended behaviour and/or an attacker calling the initialize function, gaining control of core permissions and functions, as highlighted in the...

Depositors might not be able to withdraw from StabilityPool if any collateral's price drops significantly

Lines of code Vulnerability details In the TroveManager contract, the closeTrove function contains a check to ensure there is more than one trove, as shown below. TroveManager.solL1278-L1282: function closeTroveaddress borrower, address collateral, Status closedStatus internal assertclosedStatus ...

Ethos stil using LUSD (not ERN) will lead to confusion and ambiguity of the protocol resulting uncountable risk for the project

Lines of code Vulnerability details Impact Ethos stil using LUSD not ERN will lead to confusion and ambiguity of the protocol resulting uncountable risk for the project Proof of Concept Ethos is a fork project of Liquity with additional changes, supporting multi collateral tokens. There are some...

STRATEGIST still can assign a higher percentage into the withdrawMaxLoss, which may lead to a very higher risk for users to lose the principle sent as a collateral

Lines of code Vulnerability details Impact Within the ReaperVaultV2updateWithdrawMaxLoss above, the user who has a STRATEGIST role still can assign a higher percentage i.e. 99% = 9999 into the withdrawMaxLoss. This lead to a very higher risk for users to lose the principle sent as a collateral if...

The _allocBPS field from strategy struct can be 0 this will make the strategy irrevocable

Lines of code Vulnerability details Impact No sanity check will make a strategy irrevocable Proof of Concept The addStrategy function can be used to add a strategy, there are multiple sanity checks inside the function however there is missing a very important one, there is no check for allocBPS !...

Inexistent Slippage Evaluation

Lines of code Vulnerability details Impact The ecosystem of Ethos Reserve contains an EIP-4626 implementation of a vault meant to be integrated by its LUSD lending and borrowing system. As per the standard's Security Considerations itself, slippage checks need to be introduced at the integration...

Users would lose some shares during withdrawal in ReaperVaultV2._withdraw().

Lines of code Vulnerability details Impact ReaperVaultV2.withdraw burns 100% of shares even if the vault balance is less than the required underlying amount. As a result, users would lose some shares during withdrawal. Proof of Concept Users can receive underlying tokens by burning their shares...

ReaperVaultERC4626.sol is not EIP-4626 compliant

Lines of code Vulnerability details Impact Other protocols integrated with Ethos Reserve may mistakenly assume that its function complies with EIP-4626. Therefore, this may lead to integration issues in the future, which could result in various problems for both parties. Proof of Concept All...

Inexistent Prevention of Duplicates

Lines of code Vulnerability details Impact The CollateralConfig::initialize function which instantiates the contract's state does not prevent duplicate collateral entries from being specified which can occur undetected. If the system is setup with duplicate collateral entries, the Ethos Core...

Lack of sanity check could block the vault

Lines of code Vulnerability details Impact All deposits will be blocked Proof of Concept It is possible to set tvlCap as 0 as there are no sanity checks to prevent that, if that would happen to vault will be locked for deposits as there is a check at line 324 in deposit function that will check t...

Address(0) check on array of addresses is not performed

Lines of code Vulnerability details Impact Likelihood-Impact = Severity Low-High = Medium Proof of Concept function initialize address vault, address memory strategists, address memory multisigRoles,// @audit array both length check IAToken gWant calls to another ReaperBaseStrategyinit in contrac...

Attacker contract can avoid being blocked

Lines of code Vulnerability details Impact A Malicious attacker can interact with the system and selfdestruct his own contract then use CREATE2 to recreate it at same address when he needs to interact with the system again. Proof of Concept Tools Used Manual Review Recommended Mitigation Steps...

ERC20 approve can fail for some tokens

Lines of code ReaperBaseStrategyv4.solL74 Vulnerability details Summary Some tokens like USDT do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the actual allowance must be approved. approve reverts for tokens like USDT...

Incorrect Price-Feed Failsafe System

Lines of code https://github...

pullCollateralFromBorrowerOperationsOrDefaultPool can fail on if _amount will set to zero

Lines of code Vulnerability details Impact Amount can be zero,as there is not no check in place, while collect modules do attempt to send it in such a case anyway. Some ERC20 tokens do not allow zero value transfers, reverting such attempts. As a result, a combination of a token set as a collect...

LQTYStaking.sol – Reentrancy risk in stake() unstake() functions

Lines of code Vulnerability details In the LQTY smart contract, the stake and unstake functions are used to add or remove a certain amount of LQTY tokens from a user's stake, respectively. Both functions are designed to update the user's stake and the total amount of LQTY staked while allowing...

Price manipulation attack

Lines of code Vulnerability details Impact The PriceFeed.sol was marked OOS but I believe this to be a valid issue. If the chainlink feed fails which is does from time to time, the Tellor price feed takes over no matter what the delta is compared to the chainlink price. This is an issue because t...

Re-entrancy inside BorrowOperations if collateral is ERC777

Lines of code Vulnerability details Impact Invariant violations through re-entrancy if the collateral token is ERC777 Proof of Concept If the collateral token will be an ERC777 token, a malicious user could use the safeTransfer from L231 to re-enter the BorrowOperations contract context and call...

CommunityIssuance.sol – Stability pool can manipulate time stamps with the fund function to issue more oath than appropriate.

Lines of code Vulnerability details If lastDistributionTime is set to a date in the future, it would allow the issueOath function to continue to mint tokens even after it should have stopped. This is because the if statement in issueOath checks whether the current time is greater than...

ReaperBaseStrategyv4 is not Initializable

Lines of code Vulnerability details Impact ReaperStrategyGranarySupplyOnly calls function ReaperBaseStrategyinit from ReaperBaseStrategyv4, but ReaperBaseStrategyv4 is not Initializable. If the ReaperBaseStrategyinit function is not called during contract initialization, it can cause critical...

applySingleTargetPermissions function can consume all gas

Lines of code Vulnerability details Impact An extensive array of items can be passed in the function applySingleTargetPermissions, and it can consume all gas, and the transaction can be reverted. As this is an external function, the hacker can call it directly from the DAO contract Proof of Conce...

First staker staking can lead to unfair profiting

Lines of code Vulnerability details Impact A caller is able to drain all of the fees from the pool if the caller stakes prior to totalLQTYStaked, in the LQTYStaking contract, being greater than 0. When the protocol is first deployed, FLUSD is zero. It is increased when troves are opened. If a...

_computeRewardsPerUnitStaked() may overflow

Lines of code Vulnerability details Impact no judgment that lastLUSDLossErrorOffset!=0, but debtToOffset is 0 computeRewardsPerUnitStaked may overflow Proof of Concept Use lastLUSDLossErrorOffset in the computeRewardsPerUnitStaked method to store the last excess The code is as follows: function...

Last good price might be not good if both Tellor and Chainlink prices are unreliable

Lines of code Vulnerability details Impact Calling from Pricefeed.sol, fetchPrice returns the last good price seen by the protocol whenever both Chainlink and Tellor fail to be reliable. The protocol features multi-collateral support which eventually accepts assets that are volatile in nature, du...

LUSDT non-compliance with the EIP-2612 standard

Lines of code Vulnerability details Impact Inability to integrate LUSDT into contracts of other services where the DOMAINSEPARATOR is used or strict adherence to the EIP2612 standard is required. Various network analyzers that automatically determine the type of contracts will not be able to...

Index of removed Trove is not updated

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. In the function removeTroveOwner in the TroveManager contract the Trovesborrowercollateral.arrayIndex is still equal to its previous index even though it has been removed from TroveOwners and therefore...

The design defect of the ActivePool rehypothecation mechanism, ReaperVaultV2 administrators have too much authority and have the conditions and motives to do evil, so make users lack trust in the ETHOS system (crisis of trust)

Lines of code Vulnerability details Impact In the design logic of the current system, ActivePool holds all the collaterals of active troves and deposits part of them in ReaperVaultV2 for re-mortgaging. If the deposit and withdraw functions of ReaperVaultV2 revert, Core functions such as borrowing...

Integer Overflow

Lines of code Vulnerability details Impact The owner of the ActivePool contract can set yield distribution parameters that do not add to the expected 10000 BPS. This would cause the rebalance function to send the incorrect number of tokens when using the splits. Proof of Concept Calling...

Improper Management of Critical Market Condition

Lines of code Vulnerability details Impact The BorrowerOperations::requireValidAdjustmentInCurrentMode validation function will apply a weak level of validation, causing the system to magnify its critical market conditions when in recovery mode. In detail, both...

TEST4

Lines of code L1 Vulnerability details THIS IS A TEST --- The text was updated successfully, but these errors were encountered: All reactions...

ReaperVaultV2 withdrawals can be frozen if Granary market has liquidity shortage

Lines of code Vulnerability details Summary ReaperStrategyGranarySupplyOnly do not control for liquidity squeeze that is typical for lending markets and can routinely happen in Granary markets. I.e. some funds can be temporary locked just because they are lent out. This is locked funds situation...

Integer Overflow & Underflow

Lines of code Vulnerability details Impact In the setYieldDistributionParams function, there is a danger of underflow or overflow of functionality. Owner calls the function and sets the values to be passed as uint256 for treasurySplit, SPSplit & stakingSplit. There is no check in place to ensure...

Redemption fee increase can be thwarted

Lines of code Vulnerability details Impact The mechanism to increase the baseRate during redemptions is moot, up to gas fees. Therefore an arbitrageur can redeem more than what is healthy for the supply of LUSD. This weakens the price floor at $1, and may cause needless volatility. It further...

Strategist can break the whole protocol

Lines of code Vulnerability details Background There is known attack to ERC4626 vaults, called inflation attacks. It seems that the ReaperVault is not affected since the only depositor will be the ActivePool contract. But what actually the internal function deposituint256 amount, address receiver...

[NAZ-M2] ReaperVaultERC4626.sol doesn't fully conform to EIP4626 implementation

Lines of code Vulnerability details Impact Specifically the two function maxDeposit && maxMint don't fully conform to EIP4626 implementation. Proof of Concept Looking at the following from EIP4626: This assumes that the user has infinite assets, i.e. MUST NOT rely on balanceOf of asset. This goes...

Decay interval can be extended

Lines of code Vulnerability details Impact The half-life defined by MINUTEDECAYFACTOR can be extended from 12h up to 24h. Proof of Concept minutesPassed is truncated to the minute. This means that the actual time passed may be up to a minute more than calculated. updateLastFeeOpTime is used to on...

CommunityIssuance doesn't check the return value of OathToken.transferFrom() and OathToken.transfer()

Lines of code Vulnerability details Impact The function CommunityIssuance.fund calls OathToken.transferFrom but doesn't check the return value, which indicates whether the transfer succeeded or failed. Similarly, the function CommunityIssuance.sendOath calls OathToken.transfer but doesn't check t...

Price manipulation attack

Lines of code Vulnerability details Impact The PriceFeed.sol was marked OOS but I believe this to be a valid issue. If the chainlink feed fails which is does from time to time, the Tellor price feed takes over no matter what the delta is compared to the chainlink price. This is an issue because t...

Centralization Risk for trusted owners

Lines of code https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos...

Max approve will not work for tokens that do not support it

Lines of code Vulnerability details Impact Max approve does not work for all tokens types Proof of Concept Inside the ReaperBaseStrategyv4 contract initializers, the contract is doing a max approve for the vault over the underlying asset, however this will not work for all tokens types Tools Used...