Users can bypass fees by calling receiveFromBridge to swap & execute

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-01-decent/blob/main/src/UTB.sol#L311-L319
https://github.com/code-423n4/2024-01-decent/blob/main/src/UTB.sol#L108-L125

Vulnerability details

Impact:

Any user can call the receiveFromBridge function to perform a swap & execute without paying the protocol fees, the receiveFromBridge function allows the users to invoke _swapAndExecute without the processing of fees through the retrieveAndCollectFees modifier that it's present in the swapAndExecute function. Basically the fees can be bypassed by calling receiveFromBridge instead of swapAndExecute resulting in a financial loss for the protocol.

Proof of Concept:

The swapAndExecute function allows a user to swap currency from the incoming to the outgoing token and executes a transaction with that payment, in the process the user must also pay a protocol fee which is handled by the retrieveAndCollectFees modifier :

function swapAndExecute(
    SwapAndExecuteInstructions calldata instructions,
    FeeStructure calldata fees,
    bytes calldata signature
)
    public
    payable
    retrieveAndCollectFees(fees, abi.encode(instructions, fees), signature)
{
    _swapAndExecute(
        instructions.swapInstructions,
        instructions.target,
        instructions.paymentOperator,
        instructions.payload,
        instructions.refund
    );
}

As it can be seen the function internally invokes _swapAndExecute for handling the swap & execution logic.

The issue now is that there is another function that also internally invokes _swapAndExecute which is the receiveFromBridge function:

function receiveFromBridge(
    SwapInstructions memory postBridge,
    address target,
    address paymentOperator,
    bytes memory payload,
    address payable refund
) public {
    //@audit callable by anyone
    _swapAndExecute(postBridge, target, paymentOperator, payload, refund);
}

The function is public so it's callable by anyone to perform a swap and execute a tx but this function doesn't include the retrieveAndCollectFees modifier as in swapAndExecute and so it doesn't charge fees when it's called.

In normal situations the receiveFromBridge function should only be called by the approved bridge adapters (registered by the owner) but because there is no check for that the caller is really a bridge adapter, any user can use the function instead of swapAndExecute to bypass the fees while running his swap transaction.

Tools Used:

Manual review

Recommended Mitigation Steps:

Update the receiveFromBridge function to incorporate proper access control allowing only approved bridge adapters as caller, the following check could be added:

function receiveFromBridge(
    SwapInstructions memory postBridge,
    address target,
    address paymentOperator,
    bytes memory payload,
    address payable refund
) public {
    //@audit callable only by bridge adapters
    require(bridgeAdapters[IBridgeAdapter(msg.sender).getId()] != address(0), "not bridge adapter");
    _swapAndExecute(postBridge, target, paymentOperator, payload, refund);
}

Assessed type

Access Control

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #15

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

alex-ppg changed the severity to 2 (Med Risk)