Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checkout plugin and proxy #598

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Checkout plugin and proxy #598

wants to merge 10 commits into from

Conversation

kumaryash90
Copy link
Member

No description provided.

Copy link

codecov bot commented Jan 9, 2024

Codecov Report

Attention: 23 lines in your changes are missing coverage. Please review.

Comparison is base (2568498) 64.08% compared to head (53ff9ae) 64.14%.
Report is 7 commits behind head on main.

❗ Current head 53ff9ae differs from pull request most recent head 786154c. Consider uploading reports for the commit 786154c to get more accurate results

Files Patch % Lines
...ts/unaudited/checkout/PRBProxyRegistryModified.sol 53.19% 22 Missing ⚠️
...ts/prebuilts/unaudited/checkout/TargetCheckout.sol 95.65% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #598      +/-   ##
==========================================
+ Coverage   64.08%   64.14%   +0.06%     
==========================================
  Files         215      218       +3     
  Lines        6632     6708      +76     
==========================================
+ Hits         4250     4303      +53     
- Misses       2382     2405      +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

socket-security bot commented Jan 9, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@babel/[email protected] Transitive: environment +10 191 kB nicolo-ribaudo
npm/@eslint/[email protected] filesystem, unsafe Transitive: environment, eval +26 3.73 MB eslintbot
npm/@isaacs/[email protected] None +7 184 kB isaacs
npm/@noble/[email protected] None 0 761 kB paulmillr
npm/@pkgjs/[email protected] None 0 74.2 kB oss-bot
npm/@thirdweb-dev/[email protected] environment, network 0 6.14 MB jnsdls
npm/@thirdweb-dev/[email protected] environment, eval, network Transitive: filesystem +126 295 MB jnsdls
npm/@thirdweb-dev/[email protected] environment, network Transitive: filesystem +44 3.49 MB jnsdls
npm/[email protected] None 0 52.3 kB marijn
npm/[email protected] None 0 531 kB marijn
npm/[email protected] None +7 923 kB chaijs
npm/[email protected] None 0 13.6 kB komagata
npm/[email protected] None +1 49.5 kB matteo.collina
npm/[email protected] shell Transitive: environment, filesystem +7 189 kB isaacs
npm/[email protected] filesystem Transitive: environment +10 145 kB isaacs
npm/[email protected] None +1 163 kB sindresorhus
npm/[email protected] environment +9 512 kB isaacs
npm/[email protected] None 0 285 kB isaacs
npm/[email protected] environment, filesystem 0 13.4 kB mafintosh
npm/[email protected] filesystem +2 1.27 MB isaacs
npm/[email protected] Transitive: environment, filesystem, shell, unsafe +34 4.47 MB alangpierce
npm/[email protected] environment, filesystem 0 124 kB oss-bot

🚮 Removed packages: npm/@thirdweb-dev/[email protected], npm/@thirdweb-dev/[email protected]

View full report↗︎

@kumaryash90 kumaryash90 requested a review from jakeloo January 9, 2024 21:32
Comment on lines 53 to 66
function _execute(UserOp memory op) internal {
bool success;
if (op.currency == CurrencyTransferLib.NATIVE_TOKEN) {
(success, ) = op.target.call{ value: op.valueToSend }(op.data);
} else {
if (op.valueToSend != 0 && op.approvalRequired) {
IERC20(op.currency).approve(op.target, op.valueToSend);
}

(success, ) = op.target.call(op.data);
}

require(success, "Execution failed");
}

Check failure

Code scanning / Slither

Functions that send Ether to arbitrary destinations High

Comment on lines 53 to 66
function _execute(UserOp memory op) internal {
bool success;
if (op.currency == CurrencyTransferLib.NATIVE_TOKEN) {
(success, ) = op.target.call{ value: op.valueToSend }(op.data);
} else {
if (op.valueToSend != 0 && 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 Medium

Comment on lines 53 to 66
function _execute(UserOp memory op) internal {
bool success;
if (op.currency == CurrencyTransferLib.NATIVE_TOKEN) {
(success, ) = op.target.call{ value: op.valueToSend }(op.data);
} else {
if (op.valueToSend != 0 && op.approvalRequired) {
IERC20(op.currency).approve(op.target, op.valueToSend);
}

(success, ) = op.target.call(op.data);
}

require(success, "Execution failed");
}
Comment on lines +217 to +250
function _installPlugin(IPRBProxyPlugin plugin) internal {
// Retrieve the methods to install.
bytes4[] memory methods = plugin.getMethods();

// The plugin must implement at least one method.
uint256 length = methods.length;
if (length == 0) {
revert PRBProxyRegistry_PluginWithZeroMethods(plugin);
}

// Install every method in the list.
address owner = msg.sender;
for (uint256 i = 0; i < length; ) {
// Check for collisions.
bytes4 method = methods[i];
if (address(_plugins[owner][method]) != address(0)) {
revert PRBProxyRegistry_PluginMethodCollision({
currentPlugin: _plugins[owner][method],
newPlugin: plugin,
method: method
});
}
_plugins[owner][method] = plugin;
unchecked {
i += 1;
}
}

// Set the methods in the reverse mapping.
_methods[owner][plugin] = methods;

// Log the plugin installation.
emit InstallPlugin(owner, _proxies[owner], plugin, methods);
}

Check notice

Code scanning / Slither

Reentrancy vulnerabilities Low

Comment on lines +147 to +152
function deployAndInstallPlugin(
IPRBProxyPlugin plugin
) external onlyNonProxyOwner(msg.sender) returns (IPRBProxy proxy) {
proxy = _deploy({ owner: msg.sender, target: address(0), data: "" });
_installPlugin(plugin);
}
Comment on lines +198 to +214
function _deploy(address owner, address target, bytes memory data) internal returns (IPRBProxy proxy) {
// Use the address of the owner as the CREATE2 salt.
bytes32 salt = bytes32(abi.encodePacked(owner));

// Set the owner and empty out the target and the data to prevent reentrancy.
constructorParams = ConstructorParams({ owner: owner, target: target, data: data });

// Deploy the proxy with CREATE2.
proxy = new PRBProxy{ salt: salt }();
delete constructorParams;

// Associate the owner and the proxy.
_proxies[owner] = proxy;

// Log the creation of the proxy.
emit DeployProxy({ operator: msg.sender, owner: owner, proxy: proxy });
}

Check notice

Code scanning / Slither

Reentrancy vulnerabilities Low

Reentrancy in PRBProxyRegistryModified._deploy(address,address,bytes):
External calls:
- proxy = new PRBProxy()
State variables written after the call(s):
- _proxies[owner] = proxy
- delete constructorParams
Comment on lines +137 to +144
function deployAndExecuteAndInstallPlugin(
address target,
bytes calldata data,
IPRBProxyPlugin plugin
) external override onlyNonProxyOwner(msg.sender) returns (IPRBProxy proxy) {
proxy = _deploy({ owner: msg.sender, target: target, data: data });
_installPlugin(plugin);
}
Comment on lines +137 to +144
function deployAndExecuteAndInstallPlugin(
address target,
bytes calldata data,
IPRBProxyPlugin plugin
) external override onlyNonProxyOwner(msg.sender) returns (IPRBProxy proxy) {
proxy = _deploy({ owner: msg.sender, target: target, data: data });
_installPlugin(plugin);
}
Comment on lines +147 to +152
function deployAndInstallPlugin(
IPRBProxyPlugin plugin
) external onlyNonProxyOwner(msg.sender) returns (IPRBProxy proxy) {
proxy = _deploy({ owner: msg.sender, target: address(0), data: "" });
_installPlugin(plugin);
}
Comment on lines +217 to +250
function _installPlugin(IPRBProxyPlugin plugin) internal {
// Retrieve the methods to install.
bytes4[] memory methods = plugin.getMethods();

// The plugin must implement at least one method.
uint256 length = methods.length;
if (length == 0) {
revert PRBProxyRegistry_PluginWithZeroMethods(plugin);
}

// Install every method in the list.
address owner = msg.sender;
for (uint256 i = 0; i < length; ) {
// Check for collisions.
bytes4 method = methods[i];
if (address(_plugins[owner][method]) != address(0)) {
revert PRBProxyRegistry_PluginMethodCollision({
currentPlugin: _plugins[owner][method],
newPlugin: plugin,
method: method
});
}
_plugins[owner][method] = plugin;
unchecked {
i += 1;
}
}

// Set the methods in the reverse mapping.
_methods[owner][plugin] = methods;

// Log the plugin installation.
emit InstallPlugin(owner, _proxies[owner], plugin, methods);
}

Check notice

Code scanning / Slither

Reentrancy vulnerabilities Low

Comment on lines +198 to +214
function _deploy(address owner, address target, bytes memory data) internal returns (IPRBProxy proxy) {
// Use the address of the owner as the CREATE2 salt.
bytes32 salt = bytes32(abi.encodePacked(owner));

// Set the owner and empty out the target and the data to prevent reentrancy.
constructorParams = ConstructorParams({ owner: owner, target: target, data: data });

// Deploy the proxy with CREATE2.
proxy = new PRBProxy{ salt: salt }();
delete constructorParams;

// Associate the owner and the proxy.
_proxies[owner] = proxy;

// Log the creation of the proxy.
emit DeployProxy({ operator: msg.sender, owner: owner, proxy: proxy });
}

Check notice

Code scanning / Slither

Reentrancy vulnerabilities Low

@@ -0,0 +1,251 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.8.18;

Check warning

Code scanning / Slither

Different pragma directives are used Warning

Different versions of Solidity are used:
- Version used: ['>=0.8.18', '>=0.8.4', '^0.8.0', '^0.8.1', '^0.8.10', '^0.8.11', '^0.8.12', '^0.8.2', '^0.8.4', '^0.8.8', '^0.8.9']
- >=0.8.18
- >=0.8.18
- >=0.8.4
- >=0.8.4
- >=0.8.4
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- ^0.8.0
- [^0.8.0](contracts/extension/SignatureAction.sol#L2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant