Front-Running Attacks Leads to Loss of Yield and Increased Costs

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/ffbe532c6f5224a55ce099b4016bd8806bdbc913/src/BranchPort.sol#L188-L219

Vulnerability details

Impact

The protocol is at risk of losing funds and yield due to the potential for front-running attacks that exploit the race condition. Malicious actors can choose to withdraw from strategies with higher fees or slippage, thereby increasing costs for the protocol.

Proof of Concept

The BranchPort.replenishReserves function allows anyone to withdraw tokens from a strategy to refill the reserve when it falls below the minimum.

function replenishReserves(address _strategy, address _token) external override lock {
    // Cache Strategy Token Global Debt
    uint256 strategyTokenDebt = getStrategyTokenDebt[_token];

    // Get current balance of _token
    uint256 currBalance = ERC20(_token).balanceOf(address(this));

    // Get reserves lacking
    uint256 reservesLacking = _reservesLacking(strategyTokenDebt, _token, currBalance);

    // Cache Port Strategy Token Debt
    uint256 portStrategyTokenDebt = getPortStrategyTokenDebt[_strategy][_token];

    // Calculate amount to withdraw. The lesser of reserves lacking or Strategy Token Global Debt.
    uint256 amountToWithdraw = portStrategyTokenDebt < reservesLacking ? portStrategyTokenDebt : reservesLacking;

    // Update Port Strategy Token Debt
    getPortStrategyTokenDebt[_strategy][_token] = portStrategyTokenDebt - amountToWithdraw;
    // Update Strategy Token Global Debt
    getStrategyTokenDebt[_token] = strategyTokenDebt - amountToWithdraw;

    // Withdraw tokens from startegy
    IPortStrategy(_strategy).withdraw(address(this), _token, amountToWithdraw);

    // Check if _token balance has increased by _amount
    require(
        ERC20(_token).balanceOf(address(this)) - currBalance == amountToWithdraw, "Port Strategy Withdraw Failed"
    );

    // Emit DebtRepaid event
    emit DebtRepaid(_strategy, _token, amountToWithdraw);
}

https://github.com/code-423n4/2023-09-maia/blob/ffbe532c6f5224a55ce099b4016bd8806bdbc913/src/BranchPort.sol#L188-L219

A port can have multiple strategies for a single token. Because anyone can call this function, it creates a race condition among these strategies. A malicious user can front-run the transaction and choose to withdraw from a strategy that has high withdrawal fees or slippage, thereby causing financial loss to the protocol.

Example Scenario

• Strategy A: High return but high withdrawal fee
• Strategy B: Low return and no withdrawal fee A malicious user can front-run and choose to withdraw from Strategy A, thereby incurring higher costs for the protocol and may benefit him.

Tools Used

Manual

Recommended Mitigation Steps

Restrict the ability to call the replenishReserves function to a strategy manager role within the protocol.

Assessed type

Access Control

[c4-pre-sort]

0xA5DF marked the issue as duplicate of #842

[c4-pre-sort]

0xA5DF marked the issue as sufficient quality report

[c4-judge]

alcueca marked the issue as unsatisfactory: Invalid

[thangtranth]

Hi @alcueca,

• It is not a duplicate of #842 since that issue only state that there is no modifier and it is different to the Natspec.

• Sponsor states that "There is no issue here. It is intended that the strategy can repay its own debt. We check such debt exists here:"

It is correct that strategy cannot over repay its own debt and it is not an issue, the problem here lies in which strategy the fund is withdrawn from. By allowing anyone to choose the strategy, the strategy can be withdrawn in an order that harmful to the protocol. The protocol can try to withdraw from protocol B but be front-run executed and end up withdrawing from protocol A.

[c4-judge]

alcueca marked the issue as not a duplicate

[alcueca]

The harm scenario is very hypothetical, but upgrading to QA grade-a so that the sponsor takes it into consideration.

[c4-judge]

alcueca changed the severity to QA (Quality Assurance)

[c4-judge]

alcueca marked the issue as grade-a