Skip to content

Latest commit

 

History

History
55 lines (43 loc) · 3.06 KB

M-07.md

File metadata and controls

55 lines (43 loc) · 3.06 KB

ERC20 implementations may revert on zero approval

Summary

Some ERC20 implementations revert when approve() is called with a zero amount, causing a denial of service when token allowances are reset.

Impact

Token allowances in the LAMM implementation follow a pattern in which approvals are setup for the required amount before executing the operation, and then reset back to zero after executing the operation.

The same behavior is present in Base.swap(), LiquidityPosition.mint() and LiquidityPosition.increaseLiquidity(). Taking the latter as an example, we can see the implementation calls safeApprove() with the required amount, then executes the call to increaseLiquidity() in the Uniswap NPM contract, and finally it resets the allowances back to zero by executing another call to safeApprove(), to ensure any unused allowance is cleared.

https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/libraries/LiquidityPosition.sol#L178-L204

178:     function increaseLiquidity(
179:         address token0,
180:         address token1,
181:         uint256 tokenId,
182:         uint256 amount0,
183:         uint256 amount1
184:     ) internal returns (uint128 liquidity, uint256 amount0Added, uint256 amount1Added) {
185:         // approve spending for uniswap's position manager
186:         TransferHelper.safeApprove(token0, Base.UNI_POSITION_MANAGER_ADDR, amount0);
187:         TransferHelper.safeApprove(token1, Base.UNI_POSITION_MANAGER_ADDR, amount1);
188: 
189:         // increase liquidity via position manager
190:         (liquidity, amount0Added, amount1Added) = Base.UNI_POSITION_MANAGER.increaseLiquidity(
191:             INonfungiblePositionManager.IncreaseLiquidityParams({
192:                 tokenId: tokenId,
193:                 amount0Desired: amount0,
194:                 amount1Desired: amount1,
195:                 amount0Min: 0,
196:                 amount1Min: 0,
197:                 deadline: block.timestamp
198:             })
199:         );
200: 
201:         // reset approval
202:         TransferHelper.safeApprove(token0, Base.UNI_POSITION_MANAGER_ADDR, 0);
203:         TransferHelper.safeApprove(token1, Base.UNI_POSITION_MANAGER_ADDR, 0);
204:     }

Some ERC20 implementations revert on approvals of zero value. One major example is the BNB token, that throws when called with 0 as the amount argument.

This will cause a denial of service in any of the mentioned functions, preventing the protocol from being used under the presence of such tokens.

Recommendation

There is no clean solution for this issue, a potential workaround could be to set the allowance to 1 wei:

  1. First, check the current allowance. If it is zero already, skip any modification (this will also save gas).
  2. Execute a non-reverting call to approve() using zero.
  3. If the previous call fails, then execute an approval for 1 wei.