hats-finance / Catalyst-Exchange-0x3026c1ea29bf1280f99b41934b2cb65d053c9db4

Other
1 stars 3 forks source link

_maxUnitCapacity can be breached and can crosschain receive assets action can be D0Sed #59

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

Github username: @nuthan2x Twitter username: nuthan2x Submission hash (on-chain): 0x00c42f4be17fe6e4896942e5f0a7a8cb327133064f0cac094e7ff8aa2f494c9a Severity: high

Description: Description

Attack Scenario

    function _updateUnitCapacity(uint256 Units) internal {
        uint256 MUC = _maxUnitCapacity;

        // The delta change to the limit is: timePassed · slope = timePassed · Max/decayrate
        uint256 unitCapacityReleased;
        unchecked {
            // block.timestamp > _usedUnitCapacityTimestamp, always.
            // MUC is generally low.
            unitCapacityReleased = (block.timestamp - _usedUnitCapacityTimestamp);
        }
        unitCapacityReleased *= MUC;
        unchecked {
            // DECAY_RATE != 0.
            unitCapacityReleased /= DECAY_RATE;
        }

        uint256 UC = _usedUnitCapacity; 
        // If the change is greater than the units which have passed through the limit is max
@--->   if (UC <= unitCapacityReleased) {
            if (Units > MUC) revert ExceedsSecurityLimit();
            _usedUnitCapacityTimestamp = block.timestamp;  // Set last change to block.timestamp.
            _usedUnitCapacity = Units; 
            return;
        }

        uint256 newUnitFlow = UC + Units;  // (UC + units) - unitCapacityReleased
        unchecked {
            // We know that UC > unitCapacityReleased
            newUnitFlow -= unitCapacityReleased;
        }
        if (newUnitFlow > MUC) revert ExceedsSecurityLimit();
        _usedUnitCapacityTimestamp = block.timestamp;  // Set last change to block.timestamp.
        _usedUnitCapacity = newUnitFlow;
        return;
    }

_updateUnitCapacity is called internally on receiveAsset when assets are moved crosschain. And it is called by chainInterface contract to release the liquidity or swap assets.

Flow of tx

  1. call sendAssetFixedUnit with 50% of the MUC (possible to move large units, because initially the price of Units in assets will be low on the chains)
  2. chainInterface will call receiveAsset and funds will be released
  3. Now 50% of MUC is used = UC
  4. then do the same flow but call sendAssetFixedUnit with 99% of MUC after waiting a day to decay
  5. Now 99% will also be used passing the check in the above code block of if (UC <= unitCapacityReleased), since UC is 50% of MUC, but unitCapacityReleased >= MUC after a day, and now 99% being less than 100% of MUC this tx will pass.
  6. So, now ~150% is minted. And atleast for next one day, new users can bridge because this line if (newUnitFlow > MUC) revert ExceedsSecurityLimit(); will be triggered, since newUnitFlow will be 150% > 100%, so after a day, it can be bypassed.
    1. see the logs and POC.

Attachments

  1. Proof of Concept (PoC) File
    • Paste this gist into test/ folder and run forge t --mt testPOC_MUC -vvv
[PASS] testPOC_MUC() (gas: 85083)
Logs:
  first tx: before -----------------------------------------------------------------
  _usedUnitCapacity:  0
  _usedUnitCapacityTimestamp:  0

  first tx: After -----------------------------------------------------------------
  _usedUnitCapacity:  693147180559945344
  _usedUnitCapacityTimestamp:  1706499

  second tx: before == first tx: After

  second tx: After -----------------------------------------------------------------
  _usedUnitCapacity:  1386155731683778698
  _usedUnitCapacityTimestamp:  1792900
  1. Revised Code File (Optional) will be in comments (github -issue -page)
nuthan2x commented 5 months ago

POC ==> https://gist.github.com/nuthan2x/b827a6611351cf343051fa99c0524c06

reednaa commented 5 months ago

Units have a relationship to asset value that is not linear. However, in your example we can describe it. Lets assume we have a pool of 2 vaults each with 1 asset each weighted 1. We actually don't have to make more assumptions, not even on the vault balance. We know the maxUnitCapacity for that vault, it will be ln(2). What is the value of ln(2) units? The equation used is out = balance * (1-exp(-U/W)). We know most of the values: out = balance * (1-exp(-ln(2)/1)) = balance * (1-0.5) = balance * 0.5.

Okay, so when we receive maxUnitCapacity on the target vault, exactly 50% of the balance will be bought. It doesn't matter how many number of assets are in the vault, 50% will always be bought.

Let do your case, first ln(2)/2 is bought and then ln(2) is bought. Out1 = balance * (1-exp(-ln(2)/2/1)) = balance * (1-0.7) = balance * 0.3 (1 day later) Out2 = balance*(1-0.3) * (1-exp(-ln(2)/1)) = balance * 0.7 * (1-0.5) = balance * 0.35

In total, 0,65% of the initial balance was bought, however it was over 2 days. We could actually have bought more since each day 50% of any balance is allowed to be bought.

Asterisk: In your case, the destination vault has 2 tokens. Then the maxUnitCapacity was twice what I used. How can we distribute that:

  1. All in 1 token => out = balance * (1-exp(-ln(2)*2/1)) = balance * 0.75
  2. 50% in each token => out = balance * (1-exp(-ln(2)*2/2/1)) = balance * 0.5

So you can choose between 75% of any 1 token or 50% of both tokens. In both cases, no more than 50% of the vault is withdraw.

Equation: https://github.com/hats-finance/Catalyst-Exchange-0x3026c1ea29bf1280f99b41934b2cb65d053c9db4/blob/fba322fab023a9206183fb455e9f86facd550d8a/evm/src/IntegralsVolatile.sol#L32