Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UTBExecutor.sol#execute() - The target address can gas bomb/return bomb users, forcing them to pay massive amounts of gas. #70

Closed
c4-bot-4 opened this issue Jan 21, 2024 · 6 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden primary issue Highest quality submission among a set of duplicates sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-bot-4
Copy link
Contributor

c4-bot-4 commented Jan 21, 2024

Lines of code

https://github.com/code-423n4/2024-01-decent/blob/07ef78215e3d246d47a410651906287c6acec3ef/src/UTBExecutor.sol#L52
https://github.com/code-423n4/2024-01-decent/blob/07ef78215e3d246d47a410651906287c6acec3ef/src/UTBExecutor.sol#L65
https://github.com/code-423n4/2024-01-decent/blob/07ef78215e3d246d47a410651906287c6acec3ef/src/UTBExecutor.sol#L70

Vulnerability details

Impact

The UTBExecutor contract implements a execute function, which makes an arbitrary call to a target address with a payload, and with or without a value.

    function execute(
        address target,
        address paymentOperator,
        bytes memory payload,
        address token,
        uint amount,
        address payable refund,
        uint extraNative
    ) public onlyOwner {
        bool success;
        if (token == address(0)) {
            (success, ) = target.call{value: amount}(payload);
            if (!success) {
                (refund.call{value: amount}(""));
            }
            return;
        }


        uint initBalance = IERC20(token).balanceOf(address(this));


        IERC20(token).transferFrom(msg.sender, address(this), amount);
        IERC20(token).approve(paymentOperator, amount);


        if (extraNative > 0) {
            (success, ) = target.call{value: extraNative}(payload);
            if (!success) {
                (refund.call{value: extraNative}(""));
            }
        } else {
            (success, ) = target.call(payload);
        }


        uint remainingBalance = IERC20(token).balanceOf(address(this)) -
            initBalance;


        if (remainingBalance == 0) {
            return;
        }


        IERC20(token).transfer(refund, remainingBalance);
    }

You'll notice that every time target is called, we use (success, ).

This is the same as writing (bool success, bytes memory data). success meaning if the tx reverted or not and the data being what the function returned, if anything.

In the current code data is omitted, but because of the way call works, any data that the function returns will still be copied into memory.

Memory allocation is very costly, and the target might return a massive amount of data, either maliciously or not, and the user will be forced to pay a huge amount of gas for copying it into memory, even though the return data isn't used anywhere in the execute function.

Proof of Concept

  1. Alice calls swapAndExecute.
  2. UTB performs the swap through Uniswap and forwards the call to execute inside UTBExecutor.
  3. The target address' fallback function gets triggered and he returns a huge amount of data.
  4. The data gets copied into memory, massively increasing the gas that Alice has to pay in order to execute the tx.
  5. The malicious target gas griefed Alice.

Tools Used

Manual Review

Recommended Mitigation Steps

Use a low-level assembly calls and do not copy the return data to memory.
Calculating the gas cost will also negate a 63/64 attack.

          assembly {
                success := call(/gas/, target, /value if any/, /payload if any/, /payload size if any/, 0, 0)
          }

Assessed type

call/delegatecall

@c4-bot-4 c4-bot-4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jan 21, 2024
c4-bot-5 added a commit that referenced this issue Jan 21, 2024
@c4-pre-sort
Copy link

raymondfam marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Jan 24, 2024
@c4-pre-sort
Copy link

raymondfam marked the issue as primary issue

@c4-pre-sort c4-pre-sort added the primary issue Highest quality submission among a set of duplicates label Jan 24, 2024
@raymondfam
Copy link

This has been reported by several other bots, but the report elaborates in detail on gas griefing via data omission.

@c4-sponsor
Copy link

wkantaros (sponsor) confirmed

@alex-ppg
Copy link

alex-ppg commented Feb 2, 2024

While this and relevant submissions detail that the target can consume all gas, the caller of the function specifies the target.

There is no reason to specify a target that can consume all gas and this can occur with normal transactions on the blockchain.

An exploitation path that relies on the unbounded gas consumption of the target does exist as described in #697 and in #525, however, it has not been adequately elaborated or identified by any submission. As such, I will mark all of them as having insufficient proof to demonstrate why this is an issue.

@c4-judge c4-judge closed this as completed Feb 2, 2024
@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Feb 2, 2024
@c4-judge
Copy link

c4-judge commented Feb 2, 2024

alex-ppg marked the issue as unsatisfactory:
Insufficient proof

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden primary issue Highest quality submission among a set of duplicates sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

7 participants