Merge pull request #37 from bcnmy/fix/remediations-chainlight-BTPMQ4-001

Fix/remediations chainlight btpmq4 001

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);
 }

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,10 +130,14 @@ 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);
         }
     }
 
+    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 +148,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

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
     }
 

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",