The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

An attacker can brick other attempts of executing ERC20 swaps

bauchibred

Proof of Concept

Using this search command: https://github.com/search?q=repo%3ACyfrin%2F2023-12-the-standard+safeApprove+NOT+language%3AMarkdown&type=code we can see that there is one instance of using safeApprove whenever swaps are meant to occur. Note that this issue i believe is a tad different from the already publicly disclosed from here, as functionality still works well and protocol might not have decided to implement a fix.

Take a look at SmartVaultV3.sol#L196-L204

function executeERC20SwapAndFee(ISwapRouter.ExactInputSingleParams memory _params, uint256 _swapFee) private {
IERC20(_params.tokenIn).safeTransfer(ISmartVaultManagerV3(manager).protocol(), _swapFee);
//@audit
IERC20(_params.tokenIn).safeApprove(ISmartVaultManagerV3(manager).swapRouter2(), _params.amountIn);
ISwapRouter(ISmartVaultManagerV3(manager).swapRouter2()).exactInputSingle(_params);
IWETH weth = IWETH(ISmartVaultManagerV3(manager).weth());
// convert potentially received weth to eth
uint256 wethBalance = weth.balanceOf(address(this));
if (wethBalance > 0) weth.withdraw(wethBalance);
}

Now here is the implementation of safeApprove()

function safeApprove(
IERC20 token,
address spender,
uint256 value
) internal {
// safeApprove should only be called when setting an initial allowance,
// or when resetting it to zero. To increase and decrease it, use
// 'safeIncreaseAllowance' and 'safeDecreaseAllowance'
require(
@> (value == 0) || (token.allowance(address(this), spender) == 0),
"SafeERC20: approve from non-zero to non-zero allowance"
);
_callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, value));
}

From attached code snippets, we can see that the swap uses safeApprove() which requires the allowance to be 0 otherwise it reverts, this means that as long as the allowance has not been used up sometime before, then this method will not work properly anymore.

Allowance might not be used in different instances, to showcase a potential case of how approval would be left over, when UniswapV3Router executes exactInput() If the paid token is weth, and the UniswapV3Router has enough contract balance, it will use the contract balance directly, and will not execute transferFrom(), i.e. it will not use the allowance.

UniswapV3Router transfers the payment token via pay() with the following code

https://github.com/Uniswap/v3-periphery/blob/6cce88e63e176af1ddb6cc56e029110289622317/contracts/base/PeripheryPayments.sol#L58-L62

function pay(
address token,
address payer,
address recipient,
uint256 value
) internal {
@> if (token == WETH9 && address(this).balance >= value) {
// pay with WETH9
IWETH9(WETH9).deposit{value: value}(); // wrap only what is needed to pay
IWETH9(WETH9).transfer(recipient, value);
} else if (payer == address(this)) {
// pay with tokens already in the contract (for the exact input multihop case)
TransferHelper.safeTransfer(token, recipient, value);
} else {
// pull payment
TransferHelper.safeTransferFrom(token, payer, recipient, value);
}
}

This now allows a malicious user to frontrun owners attempt to swap, they might wait for the value of swap being relatively small so they don't spend too much then front run with a transfer of ETH to UniswapV3SwapRouter.
resulting in the allowance not being used, and the next swap that requires an execution safeApprove() will surely REVERT.

Subsequent calls can now no longer work properly.

Impact

If for whatever reason the router does not use up all the current allowances, for example after exactInput() is executed, then there is a potential failure of all next attempts of ERC20 swaps.

Recommended Mitigation Steps

If safeApprove() is still going to be used, then always reset allowance to 0 before making current approval or instead just use safeIncreaseAllowance() instead.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

Support

FAQs

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