Users can use the protocol freely without paying any fees by calling the DecentEthRouter::bridgeWithPayload() function directly.

[c4-bot-1]

Lines of code

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L148-L194

Vulnerability details

The execution flow of bridgeAndExecute function

To understand the vulnerability, we need to understand the execution flow of the bridgeAndExecute() function, at least a small portion of it.

When the user wants to bridge tokens of him and execute an action on another chain, he will need to execute the UTB::bridgeAndExecute() function.

Suppose the user exists in Polygon, he has USDC and he wants to mint an NFT in Optimism which costs 1000 DAI. What will happen is that the protocol will first, in polygon, swap the user's USDC with WETH, then bridge the WETH to Optimism, then swap the WETH with DAI and then execute the arbitrary call the user wants to execute, which will be to mint the NFT in exchange for the resulting 1000 DAI from the post-bridge swap operation.

When this function is called, the following will happen:

1. Step 1: When the user calls the UTB::bridgeAndExecute() function, it will do three things: first, it will collect the fees by calling the UTBFeeCollector:collectFees() function, secondly, it will conduct the pre-bridge swap operation (occurs in the source destination), it will swap the user's USDC to WETH. thirdly, it will modify the swapInstructions which the user supplied to prepare for the post-bridge swap. Then after all of the 3 operations take place, it will invoke the UTB::callBridge() function.

2. Step 2: In the UTB::callBridge() function, some approvals are granted to the DecentBridgeAdapter contract, and then the it will invoke the function DecentBridgeAdapter::bridge() in the DecentBridgeAdapter contract.

3. Step 3: In the DecentBridgeAdapter::bridge() function, some data like the post-bridge swap payload and bridge payload (what to execute when the TX reaches destination) will be encoded, then it will reach out to the DecentEthRouter contract and invoke the function DecentEthRouter::bridgeWithPayload

4. Step 4: When the execution reaches the DecentEthRouter::bridgeWithPayload function, an internal function containing the actual logic, with the same name will also be called: DecentEthRouter::_bridgeWithPayload

   Note: Notice that the `DecentEthRouter::bridgeWithPayload() function isn't protected by any modifiers, any body can call it directly

5. Step 5: When the execution gets inside the DecentEthRouter::_bridgeWithPayload function, the function will prepare the LzCallParams for the layerzero call and the actual bridging will happen when the dcntEth::sendAndCall function is actually invoked.

6. Step 6: The bridging process kickstarts and the execution flow is continued in the destination chain.

Here is a graph of the execution flow

The vulnerability & PoC

The vulnerability is that any user can conduct the pre-bridge swap operation using uniswap by himself, prepare the right calldata and call the function DecentEthRouter::bridgeWithPayload directly (since anybody can call it), doing so will allow the user to bypass the fee collection process completely.

The fee collection as mentioned in the previously detailed execution flow, happens when the user first calls the bridgeAndExecute() function. But as I mentioned, nothing really forces him to start execution from there. All that function does is collect the fees, conduct the pre-bridge swap operation and prepare the proper call data. The user can conduct the pre-bridge swap operation himself, prepare the proper calldata and talk directly to the DecentEthRouter::bridgeWithPayload function, effecitvely bypassing fee collection.

Anybody can call the DecentEthRouter::bridgeWithPayload directly, and the protocol has no mechanism of determining whether or not the user is using the protocol with or without paying fees.

Impact

Users can use the protocol without paying any fees.

Tools Used

Manual Audit

Recommended Mitigation Steps

Tighten up the access control on the DecentEthRouter::bridgeWithPayload function. Allow only the DecentBridgeAdapter to call the bridgeWithPayload() function.

---

Found & reported by: sin1st3r__

Team: NPCsCorp

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-pre-sort]

raymondfam marked the issue as not a duplicate

[c4-pre-sort]

raymondfam marked the issue as duplicate of #51

[raymondfam]

This report details a clear flow on how bridge dodging can be used to avoid fees.

[c4-pre-sort]

raymondfam marked the issue as high quality report

[c4-judge]

alex-ppg marked the issue as selected for report

[alex-ppg]

The Warden has demonstrated that it is possible to bypass Decent fees when performing bridging operations by directly interacting with the DecentEthRouter. This is indeed a valid concern and has been confirmed by the Sponsor.

I believe a severity of medium is more appropriate given that uncaptured profit is solely affected.

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

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

[mcgrathcoutinho]

Hi @alex-ppg, thank you for the detailed response.

This issue is invalid since the decent bridge does not charge any bridge fees in the first place. If we look at the modifier retrieveAndCollectFees, it uses the fees.feeAmount (which is the swap fee) while the fees.bridgeFee is not considered. This is expected since the decent bridge is not charging fees.

We can see this expected behaviour here. When the bridgedAmount() function is called on the adapter, the decentBridgeAdapter just returns the same amount without any bridge fees applied (see here), while the stargateBridgeAdapter applies the bridgeFee correctly here.

Please consider re-evaluating this issue.

Thank you.

[stalinMacias]

This issue is invalid since the decent bridge does not charge any bridge fees in the first place.

That doesn't make any sense. In the bypassed retrieveAndCollectFees function natspec, it is clearly saying: "Transfers fees from the sender to UTB, and finally to the Fee Collector"
And it does in fact execute a transferFrom TX to transfer the fees.feeAmount count of fees.feeToken to the UTB Fee collector (which is supposed to be holding fees).

Additionally, if we look at the docs, here is it's saying:

Do you understand from all this that there are no fees being collected?

[mcgrathcoutinho]

Hi @stalinMacias, thank you for the comment.

The fees.feeAmount is the swap fee and not the bridge fee. We can see this in the FeeStructure struct below:

struct FeeStructure {
    uint bridgeFee;
    address feeToken;
    uint feeAmount;
}

The decent bridge not taking bridge fees is something that the sponsor had confirmed as well (although in my private thread). It would be good if the judge clarifies this separately with the sponsor since it would be against the code of conduct for me to introduce information from the private thread (though I'm willing to invite anyone to it for proof).

[stalinMacias]

@mcgrathcoutinho Let's break this down.

1. The decent bridge contract per se may/or may not charge a fee that will be received directly into the contract, but one thing is taking fees when a contract is called, and another very different thing is charging fees for using a service..

• As per the Decent's documentation, the only operation that can be performed without charging fees are direct transactions, any of the other services will charge a fee.
  

2. All the parameters that are supplied to the UTB.bridgeAndExecute() function are generated by an API/backend of the protocol, why do we know this? Because in the retrieveAndCollectFees() modifier all the inputs are sent to the FeeCollector.collectFees() function to be verified that they match with the signature that was signed with an account controlled by the protocol.

• That means, the API that generates the signatures and calldatas that will be sent to the UTB contract to use any of their services will compute the corresponding fees that will be charged depending on the type of operation the user requests to perform.

3. The amount of fees computed in the FeeStructure.feeAmount represents the amount of fees that the user will be charged for the usage of the protocol's services, again, such feeAmount was previously computed by their API that will generate a signature to validate that all the inputted parameters are correct and have not been altered.

From your comment: "The fees.feeAmount is the swap fee and not the bridge fee. "

• That is a totally incorrect assumption, feeAmount doesn't represent the swap fee, feeAmount represents the fees that the protocol will charge for their services.

From your comment: "The decent bridge not taking bridge fees is something that the sponsor had confirmed as well "

• Again, one thing is that the bridge itself charges fees for its usage, like the Stargate Bridge, anybody who uses that bridge will pay a fee that will be stored in that contract. And another very different thing is charging fees for a service provided by the Decent Protocol. As per the Decent's documentation, all of their services charges fees (except the direct executions), those fees are what is represented in the FeeStructure.feeAmount .

• At this point, I've already made a clear explanation about the FeeStructure. Now, reiterating the point of this report, by directly calling the DecentEthRouter::bridgeWithPayload() function, users will be able to use the protocol's service bridgeSwapAndExecute without paying fees to the protocol for the usage of their infrastructure.

@mcgrathcoutinho I hope this explanation clears any doubts you may had about the report.

[alex-ppg]

Hey @mcgrathcoutinho and @stalinMacias, thank you for contributing to this thread.

Regardless of what the Sponsor shared in private discussions, all material in the documentation as well as the code of the project points to a fee being charged. The entire presence of the UTBFeeCollector (as its name implies) is to collect fees from the UTB contract. Otherwise, it would not exist in the repository and not be in the scope of the contest.

Based on the above, my original ruling on this issue stands.

[c4-sponsor]

wkantaros (sponsor) confirmed