Lucene search

K
code423n4Code4renaCODE423N4:2023-02-ETHOS-FINDINGS-ISSUES-770
HistoryMar 07, 2023 - 12:00 a.m.

Inexistent Prevention of Duplicates

2023-03-0700:00:00
Code4rena
github.com
3
vulnerability
impact
misbehavior
collateralconfig
ethos core
c4 judging
duplicate entry
high severity
proof of concept
withdrawal
exploit
staking
mitigation

Lines of code
<https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/StabilityPool.sol#L626-L635&gt;
<https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/LQTY/LQTYStaking.sol#L203-L211&gt;

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 codebase will significantly misbehave with the following indicative examples:

  • StabilityPool::getDepositorCollateralGain - The collateral gains will be counted twice for each depositor, causing them to withdraw double the rewards on a first-come-first-serve basis with late parties / the last individual being unable to withdraw at all
  • LQTYStaking::_getPendingCollateralGain - The pending collateral gains will be counted twice for each depositor here as well in a similar way to the above scenario

Based on historical C4 judging of duplicate-entry findings that affect assets, this finding is at minimum a medium-severity issue. Given that the issue can remain undetected during the contract’s initial setup and is only identified during withdrawals rather than deposits, it is conceivable that assets would be potentially compromised with a higher likelihood than the duplicate-entry finding referenced by this exhibit.

As a result, I have classified this w/ a high-severity given its significant impact and possibility to exploit without privileged access.

Proof of Concept

Here is a demonstration of how the vulnerability will cause the last staker to be unable to withdraw their stake:

contract DuplicateEntryExploit {
    /**
     * Simplified Assumptions: 
     *
     * - Contract Possesses 1e18 LQTY Token Units
     * - Contract Possesses 1e18 Collateral Token Units
     * - Contract Is Owner of CollateralConfig
     * - Contract Is Trove Manager / Redemption Helper / Active Pool of LQTYStaking
     * - Staking Contract Has Been Initialized Properly
     */
    function demonstrate(IERC20 collateral, ILQTYStaking staking, ICollateralConfig config) external {
        // Flaw: Initialize CollateralConfig w/ Duplicate Entry
        config.initialize([collateral, collateral], [2 ether, 2 ether], [2 ether, 2 ether]);

        // Step 1: Stake LQTY in LQTYStaking
        staking.lqtyToken().approve(address(staking), 1 ether);
        staking.stake(1 ether);

        // Step 2: Transfer Rewards & Report Gain
        collateral.transfer(address(staking), 1 ether);
        staking.increaseF_Collateral(address(collateral), 1 ether);

        // Step 3: Attempt Withdrawal of Rewards
        try staking.unstake(1 ether) {
            revert("Unreachable");
        } catch (bytes memory e) {
            revert("ISSUE: Withdrawal Failed");
        }
    }
}

The flow above demonstrates how a withdrawal is impossible as the user is β€œowed” twice the amount available within the contract. The sample can be expanded to showcase the double reward flow as well by expanding the total stakers in the system.

Tools Used

Manual review.

Recommended Mitigation Steps

We advise duplicate entries to be prohibited entirely via a require check in CollateralConfig::initialize ensuring that config.allowed is false on each iteration, thereby preventing already initialized entries from being overwritten and ultimately prohibiting duplicate entries in the collaterals data entry of the contract.


The text was updated successfully, but these errors were encountered:

All reactions