Summary
Certain ERC20 tokens, such as USDC, have the ability to blacklist addresses, preventing them from sending or receiving tokens. In the current withdrawal implementation, funds are always sent back to msg.sender, which can cause issues if the address is blacklisted, leading to stuck collateral. To mitigate this, the withdrawal function should allow users to specify an alternative receiver address.
Vulnerability Details
When an account is blacklisted by an ERC20 token contract (e.g., USDC), it cannot send or receive tokens. The current withdrawal function in the protocol sends funds directly back to msg.sender. If msg.sender is blacklisted, the transaction will revert, causing the collateral to be stuck in the protocol.
Key Points:
-
Current Behavior:
Withdrawal function sends funds back to msg.sender.
If msg.sender is blacklisted, the transaction reverts.
-
Issue:
-
Desired Behavior:
Allow users to specify a different receiver address.
If no receiver address is specified, default to msg.sender.
function withdrawMargin(uint128 tradingAccountId, address collateralType, uint256 amount) external {
@audit>> we can't pass in a address receiver>>>
MarginCollateralConfiguration.Data storage marginCollateralConfiguration =
MarginCollateralConfiguration.load(collateralType);
TradingAccount.Data storage tradingAccount =
TradingAccount.loadExistingAccountAndVerifySender(tradingAccountId);
UD60x18 amountX18 = marginCollateralConfiguration.convertTokenAmountToUd60x18(amount);
_requireAmountNotZero(amountX18);
_requireEnoughMarginCollateral(tradingAccount, collateralType, amountX18);
tradingAccount.withdraw(collateralType, amountX18);
(UD60x18 requiredInitialMarginUsdX18,, SD59x18 accountTotalUnrealizedPnlUsdX18) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18_ZERO);
SD59x18 marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18);
tradingAccount.validateMarginRequirement(requiredInitialMarginUsdX18, marginBalanceUsdX18, UD60x18_ZERO);
@audit>> transfer only to msg.sender>>> IERC20(collateralType).safeTransfer(msg.sender, amount);
emit LogWithdrawMargin(msg.sender, tradingAccountId, collateralType, amount);
}
Impact
This issue can lead to a denial of service for users with blacklisted addresses, preventing them from withdrawing their collateral. Based on docs -
https://docs.zaros.fi/overview/resources/faq#:~:text=If your trade position has not opened yet yes. If the trade position is opened you need to wait until the end of the trade or close manually for your money to show in your balance and be available for withdrawal.
We should be able to withdraw all funds available in our balance. Not being able to do so for any reason will lead to funds being stuck in contract.
Tools Used
Recommendations
Update Withdrawal Function: Modify the withdrawal function to accept an optional receiver address. If no receiver address is provided, default to msg.sender.
-- function withdrawMargin(uint128 tradingAccountId, address collateralType, uint256 amount) external {
++ function withdrawMargin(uint128 tradingAccountId, address collateralType,address Receiver, uint256 amount) external {
++ if (Receiver==address(0)){
++ Receiver=msg.sender;
++ }
MarginCollateralConfiguration.Data storage marginCollateralConfiguration =
MarginCollateralConfiguration.load(collateralType);
TradingAccount.Data storage tradingAccount =
TradingAccount.loadExistingAccountAndVerifySender(tradingAccountId);
UD60x18 amountX18 = marginCollateralConfiguration.convertTokenAmountToUd60x18(amount);
_requireAmountNotZero(amountX18);
_requireEnoughMarginCollateral(tradingAccount, collateralType, amountX18);
tradingAccount.withdraw(collateralType, amountX18);
(UD60x18 requiredInitialMarginUsdX18,, SD59x18 accountTotalUnrealizedPnlUsdX18) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18\_ZERO);
SD59x18 marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18);
tradingAccount.validateMarginRequirement(requiredInitialMarginUsdX18, marginBalanceUsdX18, UD60x18\_ZERO);
-- IERC20(collateralType).safeTransfer(msg.sender, amount);
++ IERC20(collateralType).safeTransfer(Receiver, amount);
emit LogWithdrawMargin(msg.sender, tradingAccountId, collateralType, amount);
}
