code-423n4 / 2024-01-decent-findings

3 stars 3 forks source link

Malicious users could provide arbitrarily encoded bytes for swapPayload, leading to arbitrary token transfers via swappers. #554

Closed c4-bot-2 closed 8 months ago

c4-bot-2 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-01-decent/blob/07ef78215e3d246d47a410651906287c6acec3ef/src/UTB.sol#L318 https://github.com/code-423n4/2024-01-decent/blob/07ef78215e3d246d47a410651906287c6acec3ef/src/UTB.sol#L181-L184 https://github.com/code-423n4/2024-01-decent/blob/07ef78215e3d246d47a410651906287c6acec3ef/src/UTB.sol#L259-L274 https://github.com/code-423n4/2024-01-decent/blob/07ef78215e3d246d47a410651906287c6acec3ef/src/UTB.sol#L108-L124

Vulnerability details

Vulnerability Overview

Calldata Validation

Issue Overview

The bridgeAndExecute and swapAndExecute functions on the UTB contract allow users to provide arbitrary swap and bridging configuration plus destination chain calldata. This could lead to malicious users tricking others into unintentionally executing transactions via Decent's protocol.

The issue is these functions rely on the input structs containing expected and properly formatted data to integrate across chains with swappers and bridges. However there is no validation on the SwapInstructions, BridgeInstructions or encoded swapPayload.

For example, a user could provide any arbitrarily encoded bytes for swapPayload and unintended execution logic on the swappers. Or set super high swap amounts in SwapParams causing loss of funds.

Impact

If unchecked user input is passed directly to swappers and bridges, the following could occur by a malicious user:

Trigger & Examples

Exploiting requires the attacker to convince a target user to call bridgeAndExecute or swapAndExecute with malicious inputs. Example attack flows:

Which lines of code

The key lines that need input validation added are where external data gets passed directly into swappers and bridges without sanity checks: UTB.sol#L318 UTB.sol#L181-184 UTB.sol#

// UTB.sol
_swapAndExecute(postBridge, target, paymentOperator, payload, refund);

// in swapAndModifyPostBridge
SwapParams memory newPostSwapParams = abi.decode(
    instructions.postBridge.swapPayload,
    (SwapParams)
); 

// in various adapter bridge functions
SwapParams = abi.decode(postBridge.swapPayload, (SwapParams))

Proof of Concept

The swapAndExecute and bridgeAndExecute functions in UTB.sol are affected. These serve as the main entry points for users to integrate with Decent's swappers and bridges for cross-chain transactions.

Expected Behavior

These functions expect the SwapInstructions and BridgeInstructions structs to contain properly encoded calldata for the target swappers and bridges. The swappers and bridges in turn rely on the passed SwapParams and other data to be valid and not arbitrarily manipulated.

Root Cause Breakdown

  1. There is no input validation on the swapInstructions and bridgeInstructions parameters in UTB.

  2. Similarly, the encoded swapPayload bytes passed to swappers is not checked if it conforms to the expected SwapParams schema.

  3. When passed unchecked data, the swappers/bridges blindly decode using abi.decode and execute logic.

  4. This leads to loss of funds, stuck tokens, or execution of unintended logic depending on the manipulated parameters.

Tools Used

Vs Code

Recommended Mitigation Steps

Introduce input validation on UTB entry points against whitelist of swappers, sanity check numeric amounts, and validate decode output matches schema.

Assessed type

Invalid Validation

c4-pre-sort commented 8 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 8 months ago

raymondfam marked the issue as duplicate of #21

c4-judge commented 8 months ago

alex-ppg marked the issue as unsatisfactory: Invalid