Skip to content

Latest commit

 

History

History
114 lines (73 loc) · 3.42 KB

018.md

File metadata and controls

114 lines (73 loc) · 3.42 KB

Flaky Merlot Parrot

High

Excessive Refund Handling Will Cause Financial Loss for the Contract as Malicious Actors Exploit Unchecked Refunds

Summary

The unchecked handling of tokenInRefund during refund execution in Brackets.sol: performUpkeep() will cause financial loss for the contract as malicious actors can manipulate refund amounts to receive excessive tokens.

https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/automatedTrigger/Bracket.sol#L137-L141

Root Cause

In Brackets.sol: performUpkeep(), the following code:

//Brackets.sol: performUpkeep()
if (tokenInRefund != 0) {
    order.tokenIn.safeTransfer(order.recipient, tokenInRefund);
}

processes refunds without validating whether tokenInRefund exceeds order.amountIn, allowing refunds larger than the initial token input.

Internal pre-conditions

  1. Manipulated Refund Calculation:
  • The tokenInRefund value is calculated in the execute() function, which lacks explicit safeguards against over-calculation.
  1. No Validation in Refund Execution:
  • The performUpkeep() function transfers tokenInRefund directly without checking against order.amountIn.

External pre-conditions

1- Manipulated Execution Logic:

  • External contracts or malicious actors manipulate the token flow to inflate tokenInRefund during the swap process.

Attack Path

  1. A malicious actor places an order with:
  • A small amountIn (e.g., 10 tokens).
  1. The execute() function calculates a large tokenInRefund value through manipulation.
  2. During performUpkeep, the following vulnerable code executes:
if (tokenInRefund != 0) {
    order.tokenIn.safeTransfer(order.recipient, tokenInRefund);
}
  • tokenInRefund exceeds order.amountIn, resulting in an excessive refund.

4- The attacker receives more tokens than they initially provided, causing a financial loss to the contract.

Impact

Excessive Refunds: Refunds larger than order.amountIn deplete contract funds.

Financial Loss: The contract or other users suffer financial losses due to manipulated refunds.

System Trust Issues: Unchecked refunds undermine user trust in the platform's reliability and fairness.

PoC

Vulnerable Logic

  1. performUpkeep() (Refund Execution):
//performUpkeep()
if (tokenInRefund != 0) {
    order.tokenIn.safeTransfer(order.recipient, tokenInRefund);
}
  • Issue: No validation ensures that tokenInRefund does not exceed order.amountIn.

2- execute() (Refund Calculation):

//execute() 
uint256 finalTokenIn = tokenIn.balanceOf(address(this));
tokenInRefund = amountIn - (initialTokenIn - finalTokenIn);
  • Issue: If finalTokenIn is manipulated (e.g., through external contract calls), tokenInRefund can be artificially inflated.

  1. Deploy a modified version of an external contract used in execute() that manipulates token balances.

  2. Place an order with:

  • amountIn = 10 tokens.

3 . Manipulate finalTokenIn in the external contract to create an inflated tokenInRefund:

  • tokenInRefund = 50 tokens.
  1. Observe the vulnerable refund logic in performUpkeep() transferring the manipulated amount.

Result:

The recipient receives 50 tokens instead of the initial 10 tokens.

Mitigation

Proposed Fix

Add a check to ensure tokenInRefund does not exceed order.amountIn:

+ require(tokenInRefund <= order.amountIn, "Refund exceeds initial amount");