Draft
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Slither found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #570 +/- ##
==========================================
- Coverage 64.08% 63.85% -0.23%
==========================================
Files 215 220 +5
Lines 6632 6748 +116
==========================================
+ Hits 4250 4309 +59
- Misses 2382 2439 +57 ☔ View full report in Codecov by Sentry. |
Comment on lines
23
to
82
| contract Checkout is PermissionsEnumerable, ICheckout { | ||
| /// @dev Registry of vaults created through this Checkout | ||
| mapping(address => bool) public isVaultRegistered; | ||
|
|
||
| /// @dev Registry of executors created through this Checkout | ||
| mapping(address => bool) public isExecutorRegistered; | ||
|
|
||
| address public immutable vaultImplementation; | ||
| address public immutable executorImplementation; | ||
|
|
||
| constructor( | ||
| address _defaultAdmin, | ||
| address _vaultImplementation, | ||
| address _executorImplementation | ||
| ) { | ||
| vaultImplementation = _vaultImplementation; | ||
| executorImplementation = _executorImplementation; | ||
|
|
||
| _setupRole(DEFAULT_ADMIN_ROLE, _defaultAdmin); | ||
| } | ||
|
|
||
| function createVault(address _vaultAdmin, bytes32 _salt) external payable returns (address) { | ||
| bytes32 salthash = keccak256(abi.encodePacked(msg.sender, _salt)); | ||
| address vault = Clones.cloneDeterministic(vaultImplementation, salthash); | ||
|
|
||
| (bool success, ) = vault.call(abi.encodeWithSelector(Vault.initialize.selector, _vaultAdmin)); | ||
|
|
||
| require(success, "Deployment failed"); | ||
|
|
||
| isVaultRegistered[vault] = true; | ||
|
|
||
| emit VaultCreated(vault, _vaultAdmin); | ||
|
|
||
| return vault; | ||
| } | ||
|
|
||
| function createExecutor(address _executorAdmin, bytes32 _salt) external payable returns (address) { | ||
| bytes32 salthash = keccak256(abi.encodePacked(msg.sender, _salt)); | ||
| address executor = Clones.cloneDeterministic(executorImplementation, salthash); | ||
|
|
||
| (bool success, ) = executor.call(abi.encodeWithSelector(Executor.initialize.selector, _executorAdmin)); | ||
|
|
||
| require(success, "Deployment failed"); | ||
|
|
||
| isExecutorRegistered[executor] = true; | ||
|
|
||
| emit ExecutorCreated(executor, _executorAdmin); | ||
|
|
||
| return executor; | ||
| } | ||
|
|
||
| function authorizeVaultToExecutor(address _vault, address _executor) external { | ||
| require(IVault(_vault).canAuthorizeVaultToExecutor(msg.sender), "Not authorized"); | ||
| require(isExecutorRegistered[_executor], "Executor not found"); | ||
|
|
||
| IVault(_vault).setExecutor(_executor); | ||
|
|
||
| emit VaultAuthorizedToExecutor(_vault, _executor); | ||
| } | ||
| } |
Check warning
Code scanning / Slither
Contracts that lock Ether
Comment on lines
+37
to
+56
| function execute(UserOp calldata op) external { | ||
| require(_canExecute(), "Not authorized"); | ||
|
|
||
| if (op.valueToSend != 0) { | ||
| IVault(op.vault).transferTokensToExecutor(op.currency, op.valueToSend); | ||
| } | ||
|
|
||
| bool success; | ||
| if (op.currency == CurrencyTransferLib.NATIVE_TOKEN) { | ||
| (success, ) = op.target.call{ value: op.valueToSend }(op.data); | ||
| } else { | ||
| if (op.approvalRequired) { | ||
| IERC20(op.currency).approve(op.target, op.valueToSend); | ||
| } | ||
|
|
||
| (success, ) = op.target.call(op.data); | ||
| } | ||
|
|
||
| require(success, "Execution failed"); | ||
| } |
Check warning
Code scanning / Slither
Unused return
Comment on lines
+59
to
+72
| function createExecutor(address _executorAdmin, bytes32 _salt) external payable returns (address) { | ||
| bytes32 salthash = keccak256(abi.encodePacked(msg.sender, _salt)); | ||
| address executor = Clones.cloneDeterministic(executorImplementation, salthash); | ||
|
|
||
| (bool success, ) = executor.call(abi.encodeWithSelector(Executor.initialize.selector, _executorAdmin)); | ||
|
|
||
| require(success, "Deployment failed"); | ||
|
|
||
| isExecutorRegistered[executor] = true; | ||
|
|
||
| emit ExecutorCreated(executor, _executorAdmin); | ||
|
|
||
| return executor; | ||
| } |
Check notice
Code scanning / Slither
Reentrancy vulnerabilities
Comment on lines
+44
to
+57
| function createVault(address _vaultAdmin, bytes32 _salt) external payable returns (address) { | ||
| bytes32 salthash = keccak256(abi.encodePacked(msg.sender, _salt)); | ||
| address vault = Clones.cloneDeterministic(vaultImplementation, salthash); | ||
|
|
||
| (bool success, ) = vault.call(abi.encodeWithSelector(Vault.initialize.selector, _vaultAdmin)); | ||
|
|
||
| require(success, "Deployment failed"); | ||
|
|
||
| isVaultRegistered[vault] = true; | ||
|
|
||
| emit VaultCreated(vault, _vaultAdmin); | ||
|
|
||
| return vault; | ||
| } |
Check notice
Code scanning / Slither
Reentrancy vulnerabilities
Comment on lines
+43
to
+49
| function withdraw(address _token, uint256 _amount) external { | ||
| require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "Not authorized"); | ||
|
|
||
| CurrencyTransferLib.transferCurrency(_token, address(this), msg.sender, _amount); | ||
|
|
||
| emit TokensWithdrawn(_token, _amount); | ||
| } |
Check notice
Code scanning / Slither
Reentrancy vulnerabilities
Comment on lines
+96
to
+119
| function _swap(SwapOp memory _swapOp) internal { | ||
| address _tokenIn = _swapOp.tokenIn; | ||
| address _router = _swapOp.router; | ||
|
|
||
| // get quote for amountIn | ||
| (, bytes memory quoteData) = _router.staticcall(_swapOp.quoteCalldata); | ||
| uint256 amountIn; | ||
| uint256 offset = _swapOp.amountInOffset; | ||
|
|
||
| assembly { | ||
| amountIn := mload(add(add(quoteData, 32), offset)) | ||
| } | ||
|
|
||
| // perform swap | ||
| bool success; | ||
| if (_tokenIn == CurrencyTransferLib.NATIVE_TOKEN) { | ||
| (success, ) = _router.call{ value: amountIn }(_swapOp.swapCalldata); | ||
| } else { | ||
| IERC20(_tokenIn).approve(_swapOp.router, amountIn); | ||
| (success, ) = _router.call(_swapOp.swapCalldata); | ||
| } | ||
|
|
||
| require(success, "Swap failed"); | ||
| } |
Check failure
Code scanning / Slither
Functions that send Ether to arbitrary destinations
Comment on lines
+96
to
+119
| function _swap(SwapOp memory _swapOp) internal { | ||
| address _tokenIn = _swapOp.tokenIn; | ||
| address _router = _swapOp.router; | ||
|
|
||
| // get quote for amountIn | ||
| (, bytes memory quoteData) = _router.staticcall(_swapOp.quoteCalldata); | ||
| uint256 amountIn; | ||
| uint256 offset = _swapOp.amountInOffset; | ||
|
|
||
| assembly { | ||
| amountIn := mload(add(add(quoteData, 32), offset)) | ||
| } | ||
|
|
||
| // perform swap | ||
| bool success; | ||
| if (_tokenIn == CurrencyTransferLib.NATIVE_TOKEN) { | ||
| (success, ) = _router.call{ value: amountIn }(_swapOp.swapCalldata); | ||
| } else { | ||
| IERC20(_tokenIn).approve(_swapOp.router, amountIn); | ||
| (success, ) = _router.call(_swapOp.swapCalldata); | ||
| } | ||
|
|
||
| require(success, "Swap failed"); | ||
| } |
Check warning
Code scanning / Slither
Unused return
Comment on lines
+69
to
+84
| function swapAndTransferTokensToExecutor(address _token, uint256 _amount, SwapOp memory _swapOp) external { | ||
| require(_canTransferTokens(), "Not authorized"); | ||
| require(isApprovedRouter[_swapOp.router], "Invalid router address"); | ||
|
|
||
| _swap(_swapOp); | ||
|
|
||
| uint256 balance = _token == CurrencyTransferLib.NATIVE_TOKEN | ||
| ? address(this).balance | ||
| : IERC20(_token).balanceOf(address(this)); | ||
|
|
||
| require(balance >= _amount, "Not enough balance"); | ||
|
|
||
| CurrencyTransferLib.transferCurrency(_token, address(this), msg.sender, _amount); | ||
|
|
||
| emit TokensTransferredToExecutor(msg.sender, _token, _amount); | ||
| } |
Check notice
Code scanning / Slither
Reentrancy vulnerabilities
Comment on lines
+96
to
+119
| function _swap(SwapOp memory _swapOp) internal { | ||
| address _tokenIn = _swapOp.tokenIn; | ||
| address _router = _swapOp.router; | ||
|
|
||
| // get quote for amountIn | ||
| (, bytes memory quoteData) = _router.staticcall(_swapOp.quoteCalldata); | ||
| uint256 amountIn; | ||
| uint256 offset = _swapOp.amountInOffset; | ||
|
|
||
| assembly { | ||
| amountIn := mload(add(add(quoteData, 32), offset)) | ||
| } | ||
|
|
||
| // perform swap | ||
| bool success; | ||
| if (_tokenIn == CurrencyTransferLib.NATIVE_TOKEN) { | ||
| (success, ) = _router.call{ value: amountIn }(_swapOp.swapCalldata); | ||
| } else { | ||
| IERC20(_tokenIn).approve(_swapOp.router, amountIn); | ||
| (success, ) = _router.call(_swapOp.swapCalldata); | ||
| } | ||
|
|
||
| require(success, "Swap failed"); | ||
| } |
Check warning
Code scanning / Slither
Assembly usage
Comment on lines
+96
to
+119
| function _swap(SwapOp memory _swapOp) internal { | ||
| address _tokenIn = _swapOp.tokenIn; | ||
| address _router = _swapOp.router; | ||
|
|
||
| // get quote for amountIn | ||
| (, bytes memory quoteData) = _router.staticcall(_swapOp.quoteCalldata); | ||
| uint256 amountIn; | ||
| uint256 offset = _swapOp.amountInOffset; | ||
|
|
||
| assembly { | ||
| amountIn := mload(add(add(quoteData, 32), offset)) | ||
| } | ||
|
|
||
| // perform swap | ||
| bool success; | ||
| if (_tokenIn == CurrencyTransferLib.NATIVE_TOKEN) { | ||
| (success, ) = _router.call{ value: amountIn }(_swapOp.swapCalldata); | ||
| } else { | ||
| IERC20(_tokenIn).approve(_swapOp.router, amountIn); | ||
| (success, ) = _router.call(_swapOp.swapCalldata); | ||
| } | ||
|
|
||
| require(success, "Swap failed"); | ||
| } |
Check warning
Code scanning / Slither
Low-level calls
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.