Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crazy Cyan Worm - Early Withdrawals Undercharge Penalty Fees When Debond Fee <10 BPS #560

Open
sherlock-admin4 opened this issue Feb 17, 2025 · 0 comments

Comments

@sherlock-admin4
Copy link

Crazy Cyan Worm

Medium

Early Withdrawals Undercharge Penalty Fees When Debond Fee <10 BPS

Summary

The integer division in PodUnwrapLocker.earlyWithdraw's penalty calculation will cause undercollection for early withdrawals when debond fees are set below 10 basis points. The protocol loses expected penalty revenue as users avoid the 10% surcharge due to truncation errors in basis point calculations.

Root Cause

In PodUnwrapLocker.earlyWithdraw([PodUnwrapLocker.sol#L122-L139](https://www.notion.so/oioii1999/PodUnwrapLocker-earlyWithdraw-L122-L139-19cabc17a5a68014b5b1fbae49683f10)) the penalty calculation uses integer division that truncates fractional values when _debondFee < 10. This occurs because the additional 10% penalty (_debondFee / 10) rounds to zero for values below 10 basis points, combined with the DEN constant being defined as 10000 in [DecentralizedIndex.sol#L](https://www.notion.so/oioii1999/PodUnwrapLocker-earlyWithdraw-L122-L139-19cabc17a5a68014b5b1fbae49683f10)20.

    function earlyWithdraw(uint256 _lockId) external nonReentrant {
		    // ...
    
        IDecentralizedIndex.Fees memory _podFees = IDecentralizedIndex(_lock.pod).fees();
        uint256 _debondFee = _podFees.debond;

        // Penalty = debond fee + 10%
@>      uint256 _penaltyBps = _debondFee + _debondFee / 10;
        uint256[] memory _penalizedAmounts = new uint256[](_lock.tokens.length);

        for (uint256 i = 0; i < _lock.tokens.length; i++) {
            if (_lock.amounts[i] > 0) {
@>              uint256 _penaltyAmount = (_lock.amounts[i] * _penaltyBps) / 10000;
                _penaltyAmount = _penaltyAmount == 0 && _debondFee > 0 ? 1 : _penaltyAmount;
                _penalizedAmounts[i] = _lock.amounts[i] - _penaltyAmount;
                if (_penaltyAmount > 0) {
                    IERC20(_lock.tokens[i]).safeTransfer(_feeRecipient, _penaltyAmount);
                }
                IERC20(_lock.tokens[i]).safeTransfer(_msgSender(), _penalizedAmounts[i]);
            }
        }
        
        // ...
    }

while in the function __DecentralizedIndex_init of the contract DecentralizedIndex:

abstract contract DecentralizedIndex is Initializable, ERC20Upgradeable, ERC20PermitUpgradeable, IDecentralizedIndex {
    using SafeERC20 for IERC20;

@>  uint16 constant DEN = 10000;
		
		// ...

    function __DecentralizedIndex_init(
        string memory _name,
        string memory _symbol,
        IndexType _idxType,
        Config memory __config,
        Fees memory __fees,
        bytes memory _immutables
    ) internal onlyInitializing {
		    // ...
		    
        require(__fees.buy <= (uint256(DEN) * 20) / 100);
        require(__fees.sell <= (uint256(DEN) * 20) / 100);
        require(__fees.burn <= (uint256(DEN) * 70) / 100);
        require(__fees.bond <= (uint256(DEN) * 99) / 100);
@>      require(__fees.debond <= (uint256(DEN) * 99) / 100);
        require(__fees.partner <= (uint256(DEN) * 5) / 100);

        indexType = _idxType;
        created = block.timestamp;
@>      _fees = __fees;
        _config = __config;
        _config.debondCooldown = __config.debondCooldown == 0 ? 60 days : __config.debondCooldown;
        
        // ...		    
    }		    
    
    // ...
}

Furthermore, there is no other way to update DEN and _fees. The flawed calculation _debondFee + _debondFee / 10 fails to properly apply the intended 10% penalty surcharge when the base _debondFee is set below 10 bps (0.1%). This results in early withdrawals paying only the base debond fee without the protocol-specified penalty surcharge.

Internal Pre-conditions

  1. Protocol sets debond fee to 5 bps (0.05% as example) through governance
  2. DEN constant of 10000 basis points is hardcoded in system
  3. Penalty calculation uses integer division for 10% surcharge

External Pre-conditions

  1. User locks $100,000 (as example) worth of assets in PodUnwrapLocker
  2. Market conditions incentivize early withdrawal (e.g., price drop)
  3. Protocol operates with sub-10 bps debond fee

Attack Path

  1. Withdraw $100,000 during 5 bps fee period:
    • Correct penalty: 5 bps + 0.5 bps = 5.5 bps ($550)
    • Actual penalty: 5 bps ($500)
  2. Attacker repeats with 10 withdrawals of $1M each:
    • Protocol loses 0.5 bps × $10M = $5000
  3. Protocol processes 1000 withdrawals at 5 bps:
    • Total loss: 0.5 bps × $100M = $50,000

Impact

Protocol loses 9.09% of expected penalty revenue (0.5/5.5) per qualifying withdrawal. For $1B in early withdrawals at 5 bps:

  • Expected penalty: $1B × 0.055% = $550,000
  • Actual penalty: $1B × 0.05% = $500,000
  • Direct loss: $50,000 (9.09% of expected fees)

PoC

No response

Mitigation

Modify the penalty calculation to apply the 10% surcharge before division:

		uint256 penaltyAmount = (_lock.amounts[i] * debondFee * 11) / (10000 * 10);

This preserves precision by multiplying _debondFee by 11 (equivalent to 110%) before division, ensuring the 10% penalty is properly applied regardless of the debond fee value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant