Skip to content

Commit 10da95c

Browse files
authored
Silent slither reentrancy detector for IERC7579Hook.preCheck (#78)
1 parent 4f8718c commit 10da95c

File tree

1 file changed

+12
-7
lines changed

1 file changed

+12
-7
lines changed

contracts/account/extensions/AccountERC7579Hooked.sol

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,27 @@ import {AccountERC7579} from "./AccountERC7579.sol";
99
/**
1010
* @dev Extension of {AccountERC7579} with support for a single hook module (type 4).
1111
*
12-
* If installed, this extension will call the hook module's {IERC7579Hook-preCheck} before
13-
* executing any operation with {_execute} (including {execute} and {executeFromExecutor} by
14-
* default) and {IERC7579Hook-postCheck} thereafter.
12+
* If installed, this extension will call the hook module's {IERC7579Hook-preCheck} before executing any operation
13+
* with {_execute} (including {execute} and {executeFromExecutor} by default) and {IERC7579Hook-postCheck} thereafter.
14+
*
15+
* NOTE: Hook modules break the check-effect-interaction pattern. In particular, the {IERC7579Hook-preCheck} hook can
16+
* lead to potentially dangerous reentrancy. Using the `withHook()` modifier is safe if no effect is performed
17+
* before the preHook or after the postHook. That is the case on all functions here, but it may not be the case if
18+
* functions that have this modifier are overridden. Developers should be extremely careful when implementing hook
19+
* modules or further overriding functions that involve hooks.
1520
*/
1621
abstract contract AccountERC7579Hooked is AccountERC7579 {
1722
address private _hook;
1823

1924
/**
20-
* @dev Calls {IERC7579Hook-preCheck} before executing the modified
21-
* function and {IERC7579Hook-postCheck} thereafter.
25+
* @dev Calls {IERC7579Hook-preCheck} before executing the modified function and {IERC7579Hook-postCheck}
26+
* thereafter.
2227
*/
2328
modifier withHook() {
2429
address hook_ = hook();
2530
bytes memory hookData;
31+
32+
// slither-disable-next-line reentrancy-no-eth
2633
if (hook_ != address(0)) hookData = IERC7579Hook(hook_).preCheck(msg.sender, msg.value, msg.data);
2734
_;
2835
if (hook_ != address(0)) IERC7579Hook(hook_).postCheck(hookData);
@@ -50,7 +57,6 @@ abstract contract AccountERC7579Hooked is AccountERC7579 {
5057
}
5158

5259
/// @dev Installs a module with support for hook modules. See {AccountERC7579-_installModule}
53-
/// TODO: withHook? based on what value?
5460
function _installModule(
5561
uint256 moduleTypeId,
5662
address module,
@@ -64,7 +70,6 @@ abstract contract AccountERC7579Hooked is AccountERC7579 {
6470
}
6571

6672
/// @dev Uninstalls a module with support for hook modules. See {AccountERC7579-_uninstallModule}
67-
/// TODO: withHook? based on what value?
6873
function _uninstallModule(
6974
uint256 moduleTypeId,
7075
address module,

0 commit comments

Comments
 (0)