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

1 stars 1 forks source link

Yashar - Attacker can prevent the liquidation of their loan using address collision in `LockstakeEngine` #42

Closed sherlock-admin4 closed 1 month ago

sherlock-admin4 commented 1 month ago

Yashar

Medium

Attacker can prevent the liquidation of their loan using address collision in LockstakeEngine

Summary

Using create2 in LockstakeEngine.sol:242 allows attackers to prevent their collateral from being liquidated through address collision.

Root Cause

In LockstakeEngine.sol:242, the open function uses create2 to deploy the urn for the user. This function uses msg.sender and index to generate a salt, which is then used in create2 to deploy the urn. After a user opens a position and deploys an urn, they can call lock and deposit their collateral. The lock function will then mint lsmkr to the user's urn:

        lsmkr.mint(urn, wad);

In the case of a liquidation, the clipper will call LockstakeEngine.sol:428, which will burn the lsmkr from the user's urn.

Given that the open function uses create2 to deploy the urn, an attacker who can find an address collision will be able to deploy a malicious contract at the address that will collide with the urn. The attacker can approve infinite amount of lsmkr to themselves using this malicious contract and then selfdestruct it. They can then call open with the specific msg.sender and index that will result in a collision with the address of the malicious contract. After calling lock, the attacker can transfer the lsmkr to another address and draw a loan. In this situation, if the clipper calls onKick, the transaction will revert in this line because the urn has no lsmkr in balance. The attacker will be able to draw a loan and prevent the clipper from liquidating the collateral while simultaneously yielding in StakingRewards.sol with their lsmkr. This will result in bad debt for the protocol since the loan cannot be liquidated, and since the protocol cannot blacklist the urn, the attacker can repeat this many times.

Internal pre-conditions

There are no internal preconditions.

External pre-conditions

There are no external preconditions.

Attack Path

The address collisions an attacker will need to find are:

  1. One undeployed urn address.
  2. An arbitrary attacker-controlled contract.

Both sets of addresses can be brute-force searched:

Attacker finds a collision in the list (two same addresses). The attacker will then use the deployer contract with that specific salt that will result in address 0xCollide and deploy the malicious contract. This malicious contract approves an infinite amount of lsmkr to the attacker's address and selfdestruct itself in the constructor. Then, the attacker uses the second address (the msg.sender that would result in an urn with 0xCollide as the address) and calls open. The collision happens, and the attacker can now transfer the lsmkr to any other addresses.

The feasibility, as well as the detailed technique and hardware requirements of finding a collision, are sufficiently described in multiple references:

Although this would require a large amount of compute it is already possible to break with current computing. In less than a decade this would likely be a fairly easily attained amount of compute, nearly guaranteeing this attack.

Impact

The protocol suffers a bad debt because the attacker can interact with the lockstake, draw a loan, and prevent the protocol from liquidating the collateral.

PoC

While I cannot provide an actual hash collision due to infrastructural constraints, it's possible to provide a coded PoC to prove the following two properties of the EVM that would enable this attack:

Make a file named PoC.t.sol and paste the following code in it:

pragma solidity ^0.8.20;

contract Token {
    mapping(address => mapping(address => uint256)) public allowance;

    function increaseAllowance(address to, uint256 amount) public {
       allowance[msg.sender][to] += amount;
    }
}

contract InstantApprove {
    function setApprove(Token ts, uint256 amount) public {
        ts.increaseAllowance(msg.sender, amount);
    }

    function destroy() public {
        selfdestruct(payable(tx.origin));
    }
}

contract Test {
    Token public ts;
    uint256 public constant APPROVE_AMOUNT = 2e18;

    constructor() {
        ts = new Token();
    }

    function test_Collide(uint _salt) public returns (address) {
        InstantApprove ia = new InstantApprove{salt: keccak256(abi.encodePacked(_salt))}();
        address ia_addr = address(ia);

        ia.setApprove(ts, APPROVE_AMOUNT);
        ia.destroy();
        return ia_addr;
    }

    function getCodeSize(address addr) public view returns (uint) {
        uint size;
        assembly {
            size := extcodesize(addr)
        }
        return size;
    }

    function getAllowance(address from) public view returns (uint) {
        return ts.allowance(from, address(this));
    } 
}

Run the test:

forge test --mt test_Collide

Mitigation

The mitigation method is to prevent controlling over the deployed account address (or at least severely limit that). Some techniques may be:

Duplicate of #64