Malicious wallet may easily grief SWAPPER_ROLE by a significant margin leading to severe loss of funds
Summary
The wallet specified in swaps is supposed to be treated like a gnosis multisig for the purpose of the audit, which means users may send transactions themselves to this wallet before the SWAPPER_ROLEswaps.
As the wallet does not support permit functionality, it requires pre approving the protocol that is going to do the swap for tokens to do AmirX.defiSwap() or StablecoinHandler::_stablecoinSwap() from the wallet to the liquiditySafe.
As such, the owners of the wallet may frontrun the SWAPPER_ROLE and set approval of these tokens to 0, causing significant gas griefing way over the cost of the attack.
Root Cause
In AmirX:168, (bool walletResult, ) = wallet.call{value: 0}(defi.walletData); requires the wallet having approved previously the protocol that does the swap. As such, the owners of the wallet may set the approval to 0, frontrunning the SWAPPER_ROLE and making this revert.
Internal pre-conditions
None.
External pre-conditions
None.
Attack Path
User requests the SWAPPER_ROLE to perform AmirX:swap() with directional to true and swapping defi and stablecoin.
User frontruns SWAPPER_ROLE and sets approval of the wallet of the token to swap to 0.
Call reverts only here, spending a lot of gas in the meantime but reverting and not sending funds to liquiditySafe.
Impact
Swapper is griefed for many times more than the wallet. The wallet owner just spends 40-60k gas setting the approval to 0, but the SWAPPER_ROLE spends many times that as the revert happens after a lot of gas has already been spent.
PoC
None.
Mitigation
The SWAPPER_ROLE should verify the appproval on chain before everything or approve the tokens of the wallet atomically.
0x73696d616f
Medium
Malicious wallet may easily grief
SWAPPER_ROLE
by a significant margin leading to severe loss of fundsSummary
The wallet specified in swaps is supposed to be treated like a gnosis multisig for the purpose of the audit, which means users may send transactions themselves to this wallet before the
SWAPPER_ROLE
swaps.As the wallet does not support permit functionality, it requires pre approving the protocol that is going to do the swap for tokens to do
AmirX.defiSwap()
orStablecoinHandler::_stablecoinSwap()
from thewallet
to theliquiditySafe
.As such, the owners of the wallet may frontrun the
SWAPPER_ROLE
and set approval of these tokens to 0, causing significant gas griefing way over the cost of the attack.Root Cause
In
AmirX:168
,(bool walletResult, ) = wallet.call{value: 0}(defi.walletData);
requires the wallet having approved previously the protocol that does the swap. As such, the owners of the wallet may set the approval to 0, frontrunning theSWAPPER_ROLE
and making this revert.Internal pre-conditions
None.
External pre-conditions
None.
Attack Path
SWAPPER_ROLE
to performAmirX:swap()
with directional to true and swapping defi and stablecoin.SWAPPER_ROLE
and sets approval of the wallet of the token to swap to 0.Impact
Swapper is griefed for many times more than the wallet. The wallet owner just spends 40-60k gas setting the approval to 0, but the
SWAPPER_ROLE
spends many times that as the revert happens after a lot of gas has already been spent.PoC
None.
Mitigation
The
SWAPPER_ROLE
should verify the appproval on chain before everything or approve the tokens of the wallet atomically.