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

Clean Hemp Barracuda - Inaccurate Keeper Fee in Order Execution #54

Open
sherlock-admin4 opened this issue Jan 31, 2025 · 0 comments
Open
Labels
Sponsor Disputed The sponsor disputed this issue's validity

Comments

@sherlock-admin4
Copy link
Contributor

Clean Hemp Barracuda

Medium

Inaccurate Keeper Fee in Order Execution

Summary

The applicableGas is 0, and the KeepConfig has multiplier Base set to 0. This means the fee calculation would be (0 + bufferBase) * gasPrice, which ignores the actual gas used.

Therefore, the _executeOrder function has the issue, where keeper fees aren't accurately calculated based on real gas consumption. This would lead to keepers being underpaid and potentially refusing to execute orders, harming protocol functionality.

https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/periphery/contracts/MultiInvoker/MultiInvoker.sol#L433

https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/periphery/contracts/MultiInvoker/MultiInvoker.sol#L438

Root Cause

The _executeOrder function in the MultiInvoker contract miscalculates keeper fees by hardcoding applicableGas = 0, bypassing actual gas usage tracking. This leads to fees that do not reflect real transaction costs.

Internal Pre-conditions

No response

External Pre-conditions

No response

Attack Path

Scenario:

  1. User Action: Alice places a trigger order with a fee = 10 DSU.
  2. Keeper Exec: Bob calls _executeOrder, spending 80,000 gas.
  3. Fee Calculation:
    • applicableGas is 0 (hardcoded).
    • fee = (0 + keepBufferBase) * gasPrice = (0 + 10,000) * 20 gwei = 0.0002 ETH (converted to DSU).
    • Actual cost: 80,000 gas * 20 gwei = 0.0016 ETH.
  4. Outcome:
    • Keeper receives 0.0002 ETH but spent 0.0016 ETH.
    • Loss: 0.0014 ETH per transaction. Keepers stop executing orders over time.

Code Reference:

function _executeOrder(...) internal {
    _handleKeeperFee(
        KeepConfig(
            UFixed18Lib.ZERO, // multiplierBase = 0
            keepBufferBase,   // e.g., 10,000 gas
            UFixed18Lib.ZERO,
            keepBufferCalldata
        ),
        0, // ❌ Hardcoded `applicableGas = 0`
        msg.data[0:0],
        0,
        abi.encode(...)
    );
}

Impact

  • Underpaid Keepers: Fees are based on static buffers (e.g., keepBufferBase) instead of real gas usage, causing financial losses for keepers.
  • Protocol Dysfunction: Persistent underpayment disincentivizes keepers from executing orders, leading to unprocessed orders and user frustration.

PoC

No response

Mitigation

  1. Track Gas Usage: Measure gasleft() before and after order execution.
  2. Pass Actual Gas: Use the difference to calculate fees.

Fixed Code:

function _executeOrder(...) internal {
    uint256 startGas = gasleft(); // ✅ Capture initial gas

    // ... execute order logic ...

    uint256 gasUsed = startGas - gasleft(); // ✅ Calculate actual gas
    _handleKeeperFee(
        KeepConfig(...),
        gasUsed, // ✅ Pass real gas usage
        msg.data[0:0],
        0,
        abi.encode(...)
    );
}
@sherlock-admin3 sherlock-admin3 added the Sponsor Disputed The sponsor disputed this issue's validity label Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Sponsor Disputed The sponsor disputed this issue's validity
Projects
None yet
Development

No branches or pull requests

2 participants