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

fix: better validations #248

Merged
merged 1 commit into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions src/SsoAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,31 @@ contract SsoAccount is Initializable, HookManager, ERC1271Handler, TokenCallback
/// @param _data The calldata to pass along with the call.
function _executeCall(address _to, uint128 _value, bytes calldata _data) internal {
uint32 gas = Utils.safeCastToU32(gasleft());
bool success;

if (_to == address(DEPLOYER_SYSTEM_CONTRACT)) {
// Note, that the deployer contract can only be called with a "systemCall" flag.
SystemContractsCaller.systemCallWithPropagatedRevert(gas, _to, _value, _data);
bytes4 selector = bytes4(_data[:4]);
// Check that called function is the deployment method,
// the other deployer methods are not supposed to be called from the account.
// NOTE: DefaultAccount has the same behavior.
bool isSystemCall = selector == DEPLOYER_SYSTEM_CONTRACT.create.selector ||
selector == DEPLOYER_SYSTEM_CONTRACT.create2.selector ||
selector == DEPLOYER_SYSTEM_CONTRACT.createAccount.selector ||
selector == DEPLOYER_SYSTEM_CONTRACT.create2Account.selector;
// Note, that the deployer contract can only be called with a "isSystemCall" flag.
success = EfficientCall.rawCall({
_gas: gas,
_address: _to,
_value: _value,
_data: _data,
_isSystem: isSystemCall
});
} else {
bool success = EfficientCall.rawCall(gas, _to, _value, _data, false);
if (!success) {
EfficientCall.propagateRevert();
}
success = EfficientCall.rawCall(gas, _to, _value, _data, false);
}

if (!success) {
EfficientCall.propagateRevert();
}
}

Expand Down Expand Up @@ -201,7 +217,7 @@ contract SsoAccount is Initializable, HookManager, ERC1271Handler, TokenCallback
/// @dev Safely casts a uint256 to an address.
/// @dev Revert if the value exceeds the maximum size for an address (160 bits).
function _safeCastToAddress(uint256 _value) internal pure returns (address) {
if (_value > type(uint160).max) revert();
require(_value <= type(uint160).max, "Overflow");
return address(uint160(_value));
}
}
1 change: 1 addition & 0 deletions src/libraries/SessionLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ library SessionLib {
require(state.status[msg.sender] == Status.Active, "Session is not active");
TimestampAsserterLocator.locate().assertTimestampInRange(0, spec.expiresAt);

require(transaction.to <= type(uint160).max, "Overflow");
address target = address(uint160(transaction.to));

if (transaction.paymasterInput.length >= 4) {
Expand Down
5 changes: 5 additions & 0 deletions src/validators/SessionKeyValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import { IValidatorManager } from "../interfaces/IValidatorManager.sol";
import { SessionLib } from "../libraries/SessionLib.sol";
import { SignatureDecoder } from "../libraries/SignatureDecoder.sol";
import { TimestampAsserterLocator } from "../helpers/TimestampAsserterLocator.sol";

/// @title SessionKeyValidator
/// @author Matter Labs
Expand Down Expand Up @@ -54,6 +55,10 @@ contract SessionKeyValidator is IModuleValidator {
require(sessionSpec.signer != address(0), "Invalid signer (create)");
require(sessions[sessionHash].status[msg.sender] == SessionLib.Status.NotInitialized, "Session already exists");
require(sessionSpec.feeLimit.limitType != SessionLib.LimitType.Unlimited, "Unlimited fee allowance is not safe");
// Sessions should expire in no less than 60 seconds.
uint256 minuteBeforeExpiration = sessionSpec.expiresAt <= 60 ? 0 : sessionSpec.expiresAt - 60;
TimestampAsserterLocator.locate().assertTimestampInRange(0, minuteBeforeExpiration);

sessionCounter[msg.sender]++;
sessions[sessionHash].status[msg.sender] = SessionLib.Status.Active;
emit SessionCreated(msg.sender, sessionHash, sessionSpec);
Expand Down
Loading