sherlock-audit / 2024-02-rubicon-finance-judging

5 stars 3 forks source link

LTDingZhen - If an order is partially filled, the user will not receive a refund #69

Closed sherlock-admin2 closed 9 months ago

sherlock-admin2 commented 9 months ago

LTDingZhen

high

If an order is partially filled, the user will not receive a refund

Summary

If a filler chooses to partially fill a order, The input token of the submitter of the order will be transferred in full to GladiusReactor, and won't be refunded.

Vulnerability Detail

When the contract requests input tokens from Permit2, the following logic is executed:

//GladiusReactor.sol-L115
function transferInputTokens(
    ResolvedOrder memory order,
    address to
) internal override {
    permit2.permitWitnessTransferFrom(
        order.toPermit(),
        order.transferDetails(to),
        order.info.swapper,
        order.hash,
        PartialFillLib.PERMIT2_ORDER_TYPE,
        order.sig
    );
}

function toPermit() is used to extract token transfer logic from order:

//Permit2Lib.sol-L10
function toPermit(
    ResolvedOrder memory order
) internal pure returns (ISignatureTransfer.PermitTransferFrom memory) {
    return
        ISignatureTransfer.PermitTransferFrom({
            permitted: ISignatureTransfer.TokenPermissions({
                token: address(order.input.token),
                amount: order.input.maxAmount
            }),
            nonce: order.info.nonce,
            deadline: order.info.deadline
        });
}

As shown above, the maxAmount input token of the user's order is transferred to GladiusReactor, rather than the amount being filled.

When paying the exchanged tokens to the user, these input tokens are not refunded.

function _fill(ResolvedOrder[] memory orders) internal {
    uint256 ordersLength = orders.length;
    // attempt to transfer all currencies to all recipients
    unchecked {
        // transfer output tokens to their respective recipients
        for (uint256 i = 0; i < ordersLength; i++) {
            ResolvedOrder memory resolvedOrder = orders[i];
            uint256 outputsLength = resolvedOrder.outputs.length;

            for (uint256 j = 0; j < outputsLength; j++) {
                OutputToken memory output = resolvedOrder.outputs[j];
                output.token.transferFill(output.recipient, output.amount);
            }

            emit Fill(
                orders[i].hash,
                msg.sender,
                resolvedOrder.info.swapper,
                resolvedOrder.info.nonce
            );
        }
    }

    // refund any remaining ETH to the filler. 
    if (address(this).balance > 0) {
        CurrencyLibrary.transferNative(msg.sender, address(this).balance);
    }
}

Impact

A partion of input tokens of partially filled orders are locked in the contract.

Code Snippet

https://github.com/sherlock-audit/2024-02-rubicon-finance/blob/main/gladius-contracts-internal/src/lib/Permit2Lib.sol#L10

Tool used

Manual Review

Recommendation

Add a refund mechanism, or modify the Permit2Lib to make it avaliable for partial transferfrom.

sherlock-admin commented 9 months ago

2 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Invalid. This is user mistake

0xAadi commented:

Invalid: Wrong statement