Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

ParaSwapUtils.sol vulnerability that allows to steal all the funds through arbitrary execution of Paraswap calls.

ipcbenjamin

Summary
The swap function allows executing arbitrary Paraswap operation while granting unlimited token approvals to Paraswap's TokenTransferProxy. Malicious keepers can drain ALL approved tokens via crafted callData.

Vulnerability Details

File --> contracts/libraries/ParaSwapUtils.sol

function swap(address to, bytes memory callData) external {
_validateCallData(to, callData);
address approvalAddress = IAugustusSwapper(to).getTokenTransferProxy();
address fromToken;
uint256 fromAmount;
assembly { // Dangerous low-level data parsing
fromToken := mload(add(callData, 68))
fromAmount := mload(add(callData, 100))
}
IERC20(fromToken).safeApprove(approvalAddress, fromAmount); // Permanent approval
(bool success, ) = to.call(callData); // Arbitrary call execution
}

//Dangerous low-level data parsing --> There is not validation of the fromToken and fromAmount extracted from the raw calldata so that any people can spoof fromToken/fromAmount via raw calldata manipulation. Although it is true that if the hacker lies by writing fromAmount = 1000 USDC when it want to steal 1.000.000 USDC the approval would be insufficient, it is also true that if the fromAmount is the maximum, instead of 1.000.000 USDC, I mean:
type(uint256).max
the approval would be infinite and it could steal all the funds! So, any keeper just with enough knowledge to modify callData in the needed way could report fake token/amount values.
//Permanent approval --> it is true that, but that line approves Paraswap's proxy formax amount (fromAmount), and there is no allowance reset after the swap, which make dust attacks to be possible.
//Arbitrary call execution --> based on previous analisys and on the fact that _validateCallData only checks receiver address, but not the callData (no validation of token pairs for example), attackers can encode malicious swap operations in valid Paraswap calls.

Impact
It is critical, the worst impact possible, all funds would be lost and any keeper could do this, all kind of asset storaged could be stolen in just one transaction with success in great mayority of cases (based on Foundry tests). For example:

bytes memory evilCallData = abi.encodeWithSelector(
ParaswapRouter.swapExactTokensForTokens.selector,
1e18, // small amount
type(uint256).max, // Extract MAX allowance
[address(USDC), address(ATTACKER_TOKEN)],
address(this)
);
​
ParaSwapUtils.swap(PARASWAP_ROUTER, evilCallData); // Drains all USDC

This would approve all the USDC for Paraswap and the hacker would change all the funds for its token.
And any keeper could do this, even without advanced skills, just knowing how to make the proper callData.

Tools Used
* Slither --> helped me detecting arbitrary external call with tainted data.*
** Foundry --> To simulate the attack. Most of the cases it was successful and it confirmed unlimited approval retention.
* Manticore --> Symbolic verification confirmed full control of tokens.
* ChatGPT --> To help me figure out and confirming how risky the vulnerability was and to figure out possible solutions or recommendations.

Recommendations
We would have to restrict that call() function to only authorized keepers, or maybe only allow pre-approved or specific swap functions. We would have to add a require also to validate that the swap was successful For example:

function swap(address to, bytes memory callData) external {
_validateCallData(to, callData);
(address fromToken, uint256 fromAmount) = abi.decode(
abi.encodePacked(callData),
(address, uint256)
);
//validation of allowed tokens
require(_isWhiteListed[fromToken], "Not allowed token"); //define _isWhiteListed
// limited time approval
address approvalAddress = IAugustusSwapper(to).getTokenTransferProxy();
IERC20(tokenIn).safeApprove(approvalAddress, 0);
IERC20(tokenIn).safeApprove(approvalAddress, amountIn);
(bool success, ) = to.call(callData);
require(success, "Swap failed");
// revoke approval
IERC20(path[0]).safeApprove(approvalAddress, 0);
}
Updates

Lead Judging Commences

n0kto Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Admin is trusted / Malicious keepers

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point. Keepers are added by the admin, there is no "malicious keeper" and if there is a problem in those keepers, that's out of scope. ReadMe and known issues states: " * System relies heavily on keeper for executing trades * Single keeper point of failure if not properly distributed * Malicious keeper could potentially front-run or delay transactions * Assume that Keeper will always have enough gas to execute transactions. There is a pay execution fee function, but the assumption should be that there's more than enough gas to cover transaction failures, retries, etc * There are two spot swap functionalies: (1) using GMX swap and (2) using Paraswap. We can assume that any swap failure will be retried until success. " " * Heavy dependency on GMX protocol functioning correctly * Owner can update GMX-related addresses * Changes in GMX protocol could impact system operations * We can assume that the GMX keeper won't misbehave, delay, or go offline. " "Issues related to GMX Keepers being DOS'd or losing functionality would be considered invalid."

invalid_ParaswapUtils_swap_malicious_callData

This function call only reached via a function called by the keeper. So no malicious callData will be provided.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.