Summary
The contract uses the swapProgressData struct to track the progress of swaps, particularly in scenarios where swaps are performed as part of deposit, withdrawal, or position adjustment flows. However, the contract does not consistently update the swapProgressData.swapped variable after a swap is completed. This oversight can lead to incorrect tracking of swap progress, incorrect calculations of output amounts and unexpected behavior in subsequent operations.
Vulnerability Details
The swapProgressData struct is used to track the progress of swaps, including:
isCollateralToIndex: Indicates the direction of the swap (e.g., collateral to index token or vice versa).
swapped: The amount of output tokens that have already been swapped.
remaining: The amount of input tokens that remain to be swapped.
The swapProgressData.swapped variable is not consistently updated after a swap is completed. For example:
In the _runSwap function when metadata.length is 1 and the protocol is PROTOCOL.DEX, the function calls _doDexSwap(data, isCollateralToIndex) to perform a DEX swap. However, the output amount from this swap is not used to update swapProgressData.swapped. Instead, the output amount is only utilized in the context of the flow (either FLOW.DEPOSIT or FLOW.WITHDRAW) to determine the total amount processed.
function _runSwap(bytes[] memory metadata, bool isCollateralToIndex, MarketPrices memory prices) internal returns (bool completed) {
if (metadata.length == 0) {
revert Error.InvalidData();
}
if (metadata.length == 2) {
(PROTOCOL _protocol, bytes memory data) = abi.decode(metadata[0], (PROTOCOL, bytes));
if (_protocol != PROTOCOL.DEX) {
revert Error.InvalidData();
}
swapProgressData.swapped = swapProgressData.swapped + _doDexSwap(data, isCollateralToIndex);
(_protocol, data) = abi.decode(metadata[1], (PROTOCOL, bytes));
if (_protocol != PROTOCOL.GMX) {
revert Error.InvalidData();
}
_doGmxSwap(data, isCollateralToIndex);
return false;
} else {
if (metadata.length != 1) {
revert Error.InvalidData();
}
(PROTOCOL _protocol, bytes memory data) = abi.decode(metadata[0], (PROTOCOL, bytes));
if (_protocol == PROTOCOL.DEX) {
uint256 outputAmount = _doDexSwap(data, isCollateralToIndex);
if (flow == FLOW.DEPOSIT) {
_mint(counter, outputAmount + swapProgressData.swapped, true, prices);
} else if (flow == FLOW.WITHDRAW) {
_handleReturn(outputAmount + swapProgressData.swapped, false, true);
} else {
_updateState(!isCollateralToIndex, isCollateralToIndex);
}
return true;
} else {
_doGmxSwap(data, isCollateralToIndex);
return false;
}
}
}
In the afterOrderExecution function, the swapProgressData.swapped variable is not updated at all.
function afterOrderExecution(
bytes32 requestKey,
bytes32 positionKey,
IGmxProxy.OrderResultData memory orderResultData,
MarketPrices memory prices
) external nonReentrant {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
_gmxLock = false;
} else if (orderResultData.orderType == Order.OrderType.MarketSwap) {
uint256 outputAmount = orderResultData.outputAmount;
if (swapProgressData.isCollateralToIndex) {
emit GmxSwap(
address(collateralToken),
swapProgressData.remaining,
indexToken,
outputAmount,
true
);
} else {
emit GmxSwap(
indexToken,
swapProgressData.remaining,
address(collateralToken),
outputAmount,
false
);
}
if (flow == FLOW.DEPOSIT) {
_mint(counter, outputAmount + swapProgressData.swapped, false, prices);
_finalize(hex'');
} else if (flow == FLOW.WITHDRAW) {
_handleReturn(outputAmount + swapProgressData.swapped, false, false);
Impact
The swapProgressData.swapped variable is used to calculate the total output amount in functions like _mint and _handleReturn. If this variable is not updated, the calculations will be incorrect leading to overswapping or underswapping
Tools
Manual Review
Recommendations
Ensure that the swapProgressData.swapped variable is updated consistently after every swap operation, regardless of the swap type (DEX or GMX).
function _runSwap(bytes[] memory metadata, bool isCollateralToIndex, MarketPrices memory prices) internal returns (bool completed) {
//...SNIP...
if (_protocol == PROTOCOL.DEX) {
uint256 outputAmount = _doDexSwap(data, isCollateralToIndex);
+ swapProgressData.swapped = swapProgressData.swapped + outputAmount;
// update global state
if (flow == FLOW.DEPOSIT) {
// last `depositId` equals with `counter` because another deposit is not allowed before previous deposit is completely processed
- _mint(counter, outputAmount + swapProgressData.swapped, true, prices);
+ _mint(counter, swapProgressData.swapped, true, prices)
} else if (flow == FLOW.WITHDRAW) {
- _handleReturn(outputAmount + swapProgressData.swapped, false, true);
+ _handleReturn(swapProgressData.swaped, false, true)
//...SNIP...
Update the swapProgressData.swapped variable in the afterOrderExecution function after a GMX swap is completed.
function afterOrderExecution(...) external nonReentrant {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
_gmxLock = false;
//...SNIP...
if (orderResultData.orderType == Order.OrderType.MarketSwap) {
uint256 outputAmount = orderResultData.outputAmount;
if (swapProgressData.isCollateralToIndex) {
emit GmxSwap(
address(collateralToken),
swapProgressData.remaining,
indexToken,
outputAmount,
true
);
} else {
emit GmxSwap(
indexToken,
swapProgressData.remaining,
address(collateralToken),
outputAmount,
false
);
}
+ swapProgressData.swapped += outputAmount; // Update swapped amount for GMX swap
}
// Handle other order types (e.g., increase, decrease)
}
