Decent - Aymen0909's results

Lines of code

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DcntEth.sol#L20-L22

Vulnerability details

Impact:

The identified issue in the setRouter function represents a critical security vulnerability due to the absence of proper access control. As the function stands, any user can change the router address, granting them the ability to mint and burn DcntEth tokens. This lack of access control could lead to unauthorized manipulation of the protocol, posing a severe threat to the integrity of the system and potential financial losses for the protocol and users.

Proof of Concept:

The issue is evident in the setRouter function provided below:

function setRouter(address _router) public {
    //@audit Lack of access control: anyone can change the router address
    router = _router;
}

As it can be seen any user can change the router address and set it to any other address (even his own wallet) which will allow him to call mint and burn to mint or burn DcntEth tokens, this is supposed to be done only by the true router.

This issue will result in unauthorized minting of DcntEth tokens which will affect the protocol working and cause financial losses.

Tools Used:

Manual review

Recommended Mitigation Steps:

To address this issue, implement proper access control by adding the onlyOwner modifier to the setRouter function.

Assessed type

Access Control

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