Skip to content

Commit

Permalink
fixed fallbacks
Browse files Browse the repository at this point in the history
  • Loading branch information
zeroknots committed Mar 14, 2024
1 parent c9997c7 commit 8510513
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 77 deletions.
74 changes: 8 additions & 66 deletions accounts/safe7579/src/core/ModuleManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -242,74 +242,16 @@ abstract contract ModuleManager is AccessControl, Receiver, ExecutionHelper {
CallType calltype = $fallbackHandler.calltype;
if (handler == address(0)) revert NoFallbackHandler(msg.sig);

if (calltype == CALLTYPE_STATIC) {
assembly {
// When compiled with the optimizer, the compiler relies on a certain assumptions on
// how
// the
// memory is used, therefore we need to guarantee memory safety (keeping the free
// memory
// point 0x40 slot intact,
// not going beyond the scratch space, etc)
// Solidity docs:
// https://docs.soliditylang.org/en/latest/assembly.html#memory-safety
function allocate(length) -> pos {
pos := mload(0x40)
mstore(0x40, add(pos, length))
}

let calldataPtr := allocate(calldatasize())
calldatacopy(calldataPtr, 0, calldatasize())

// The msg.sender address is shifted to the left by 12 bytes to remove the padding
// Then the address without padding is stored right after the calldata
let senderPtr := allocate(20)
mstore(senderPtr, shl(96, caller()))

// Add 20 bytes for the address appended add the end
let success :=
staticcall(gas(), handler, calldataPtr, add(calldatasize(), 20), 0, 0)

let returnDataPtr := allocate(returndatasize())
returndatacopy(returnDataPtr, 0, returndatasize())
if iszero(success) { revert(returnDataPtr, returndatasize()) }
return(returnDataPtr, returndatasize())
}
}
// dis wont work. need Enum.Operation static, cause safe account emits event
// if (calltype == CALLTYPE_STATIC) {
// return _executeStaticReturnData(
// msg.sender, handler, 0, abi.encodePacked(callData, _msgSender())
// );
// }
if (calltype == CALLTYPE_SINGLE) {
assembly {
// When compiled with the optimizer, the compiler relies on a certain assumptions on
// how
// the
// memory is used, therefore we need to guarantee memory safety (keeping the free
// memory
// point 0x40 slot intact,
// not going beyond the scratch space, etc)
// Solidity docs:
// https://docs.soliditylang.org/en/latest/assembly.html#memory-safety
function allocate(length) -> pos {
pos := mload(0x40)
mstore(0x40, add(pos, length))
}

let calldataPtr := allocate(calldatasize())
calldatacopy(calldataPtr, 0, calldatasize())

// The msg.sender address is shifted to the left by 12 bytes to remove the padding
// Then the address without padding is stored right after the calldata
let senderPtr := allocate(20)
mstore(senderPtr, shl(96, caller()))

// Add 20 bytes for the address appended add the end
let success := call(gas(), handler, 0, calldataPtr, add(calldatasize(), 20), 0, 0)

let returnDataPtr := allocate(returndatasize())
returndatacopy(returnDataPtr, 0, returndatasize())
if iszero(success) { revert(returnDataPtr, returndatasize()) }
return(returnDataPtr, returndatasize())
}
return
_executeReturnData(msg.sender, handler, 0, abi.encodePacked(callData, _msgSender()));
}

// TODO: do we actually want this? security questionable...
if (calltype == CALLTYPE_DELEGATECALL) {
return _executeDelegateCallReturnData(msg.sender, handler, callData);
Expand Down
20 changes: 10 additions & 10 deletions accounts/safe7579/test/SafeERC7579.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -159,21 +159,21 @@ contract Safe7579Test is TestBaseUtil {
(uint256 ret, address msgSender, address context) = MockFallback(address(safe)).target(1337);

assertEq(ret, 1337);
assertEq(msgSender, address(safe7579));
assertEq(context, address(safe));
assertEq(msgSender, address(safe));
assertEq(context, address(this));

vm.prank(address(safe));
IERC7579Account(address(safe)).uninstallModule(
3, address(_fallback), abi.encode(MockFallback.target.selector, CALLTYPE_SINGLE, "")
);
vm.prank(address(safe));
IERC7579Account(address(safe)).installModule(
3, address(_fallback), abi.encode(MockFallback.target.selector, CALLTYPE_STATIC, "")
);
(ret, msgSender, context) = MockFallback(address(safe)).target(1337);
assertEq(ret, 1337);
assertEq(msgSender, address(safe7579));
assertEq(context, address(safe));
// vm.prank(address(safe));
// IERC7579Account(address(safe)).installModule(
// 3, address(_fallback), abi.encode(MockFallback.target.selector, CALLTYPE_STATIC, "")
// );
// (ret, msgSender, context) = MockFallback(address(safe)).target(1337);
// assertEq(ret, 1337);
// assertEq(msgSender, address(safe7579));
// assertEq(context, address(safe));

vm.prank(address(safe));
IERC7579Account(address(safe)).installModule(
Expand Down
2 changes: 1 addition & 1 deletion accounts/safe7579/test/mocks/MockFallback.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ contract MockFallback is MockFallbackBase, HandlerContext {
returns (uint256 _value, address _this, address msgSender)
{
_value = value;
msgSender = msg.sender;
_this = address(this);
msgSender = msg.sender;
}
}

0 comments on commit 8510513

Please sign in to comment.