From b5c0b04263cd7bb03b78692611f3b26f881672a1 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Thu, 14 Nov 2024 12:00:26 +0700 Subject: [PATCH 1/2] fix: remediations. allow receive eth and withdraw stuck eth --- .../common/BiconomyTokenPaymasterErrors.sol | 10 ++++++++++ contracts/token/BiconomyTokenPaymaster.sol | 17 +++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/contracts/common/BiconomyTokenPaymasterErrors.sol b/contracts/common/BiconomyTokenPaymasterErrors.sol index b77a63f..6188f11 100644 --- a/contracts/common/BiconomyTokenPaymasterErrors.sol +++ b/contracts/common/BiconomyTokenPaymasterErrors.sol @@ -75,4 +75,14 @@ contract BiconomyTokenPaymasterErrors { * @notice Throws when external signer's signature has invalid length */ error InvalidSignatureLength(); + + /** + * @notice Throws when ETH withdrawal fails + */ + error WithdrawalFailed(); + + /** + * @notice Emitted when ETH is withdrawn from the paymaster + */ + event EthWithdrawn(address indexed recipient, uint256 indexed amount); } diff --git a/contracts/token/BiconomyTokenPaymaster.sol b/contracts/token/BiconomyTokenPaymaster.sol index 75b5726..1642099 100644 --- a/contracts/token/BiconomyTokenPaymaster.sol +++ b/contracts/token/BiconomyTokenPaymaster.sol @@ -133,6 +133,10 @@ contract BiconomyTokenPaymaster is } } + receive() external payable { + // no need to emit an event here + } + /** * @dev pull tokens out of paymaster in case they were sent to the paymaster at any point. * @param token the token deposit to withdraw @@ -143,6 +147,19 @@ contract BiconomyTokenPaymaster is _withdrawERC20(token, target, amount); } + /** + * @dev Withdraw ETH from the paymaster + * @param recipient The address to send the ETH to + * @param amount The amount of ETH to withdraw + */ + function withdrawEth(address payable recipient, uint256 amount) external payable onlyOwner nonReentrant { + (bool success,) = recipient.call{ value: amount }(""); + if (!success) { + revert WithdrawalFailed(); + } + emit EthWithdrawn(recipient, amount); + } + /** * @dev pull tokens out of paymaster in case they were sent to the paymaster at any point. * @param token the token deposit to withdraw From c8943971f852acb555493f627ed95ffaf9185ebe Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:45:05 +0700 Subject: [PATCH 2/2] fix: remediations use SafeERC20.forceApprove --- contracts/token/BiconomyTokenPaymaster.sol | 3 ++- contracts/token/swaps/Uniswapper.sol | 3 ++- package.json | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/contracts/token/BiconomyTokenPaymaster.sol b/contracts/token/BiconomyTokenPaymaster.sol index 1642099..53e7cad 100644 --- a/contracts/token/BiconomyTokenPaymaster.sol +++ b/contracts/token/BiconomyTokenPaymaster.sol @@ -5,6 +5,7 @@ import { ReentrancyGuardTransient } from "@openzeppelin/contracts/utils/Reentran import { IEntryPoint } from "account-abstraction/interfaces/IEntryPoint.sol"; import { PackedUserOperation, UserOperationLib } from "account-abstraction/core/UserOperationLib.sol"; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import { IERC20Metadata } from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol"; import { BasePaymaster } from "../base/BasePaymaster.sol"; @@ -129,7 +130,7 @@ contract BiconomyTokenPaymaster is // Approve swappable tokens for max amount uint256 length = swappableTokens.length; for (uint256 i; i < length; i++) { - IERC20(swappableTokens[i]).approve(address(uniswapRouterArg), type(uint256).max); + SafeERC20.forceApprove(IERC20(swappableTokens[i]), address(uniswapRouterArg), type(uint256).max); } } diff --git a/contracts/token/swaps/Uniswapper.sol b/contracts/token/swaps/Uniswapper.sol index 2994478..9671ec8 100644 --- a/contracts/token/swaps/Uniswapper.sol +++ b/contracts/token/swaps/Uniswapper.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.27; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import "@uniswap/v3-periphery/contracts/interfaces/ISwapRouter.sol"; import "@uniswap/v3-periphery/contracts/interfaces/IPeripheryPayments.sol"; @@ -48,7 +49,7 @@ abstract contract Uniswapper { } function _setTokenPool(address token, uint24 poolFeeTier) internal { - IERC20(token).approve(address(uniswapRouter), type(uint256).max); // one time max approval + SafeERC20.forceApprove(IERC20(token), address(uniswapRouter), type(uint256).max); // one time max approval tokenToPools[token] = poolFeeTier; // set mapping of token to uniswap pool to use for swap } diff --git a/package.json b/package.json index fd27a7d..96bcd82 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "url": "https://github.com/bcnmy" }, "dependencies": { - "@openzeppelin/contracts": "5.0.2", + "@openzeppelin/contracts": "5.1.0", "@rhinestone/modulekit": "^0.4.10", "@uniswap/v3-core": "https://github.com/Uniswap/v3-core#0.8", "@uniswap/v3-periphery": "https://github.com/Uniswap/v3-periphery#0.8",