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: audit N-07 #278

Merged
merged 2 commits into from
Feb 10, 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
8 changes: 4 additions & 4 deletions src/SsoAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ contract SsoAccount is
/// @param _to The address to which the call is made.
/// @param _value The value to send along with the call.
/// @param _data The calldata to pass along with the call.
function _executeCall(address _to, uint128 _value, bytes calldata _data) internal {
function _executeCall(address _to, uint128 _value, bytes calldata _data) private {
uint32 gas = Utils.safeCastToU32(gasleft());
bool success;

Expand Down Expand Up @@ -192,7 +192,7 @@ contract SsoAccount is
/// @param _signedHash The signed hash of the transaction.
/// @param _transaction The transaction data.
/// @return The magic value if the validation was successful and bytes4(0) otherwise.
function _validateTransaction(bytes32 _signedHash, Transaction calldata _transaction) internal returns (bytes4) {
function _validateTransaction(bytes32 _signedHash, Transaction calldata _transaction) private returns (bytes4) {
// Run validation hooks
bool hookSuccess = runValidationHooks(_signedHash, _transaction);
if (!hookSuccess) {
Expand Down Expand Up @@ -222,7 +222,7 @@ contract SsoAccount is
/// @dev Increments the nonce value in Nonce Holder system contract to ensure replay attack protection.
/// @dev Reverts if the Nonce Holder stores different `_nonce` value from the expected one.
/// @param _expectedNonce The nonce value expected for the account to be stored in the Nonce Holder.
function _incrementNonce(uint256 _expectedNonce) internal {
function _incrementNonce(uint256 _expectedNonce) private {
// Allow-listing slither finding as the call's success is checked+revert within the fn
// slither-disable-next-line unused-return
SystemContractsCaller.systemCallWithPropagatedRevert(
Expand All @@ -235,7 +235,7 @@ contract SsoAccount is

/// @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) {
function _safeCastToAddress(uint256 _value) private pure returns (address) {
require(_value <= type(uint160).max, "Overflow");
return address(uint160(_value));
}
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/SessionLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ library SessionLib {
CallSpec[] memory callPolicies,
uint64[] memory periodIds,
uint256 periodIdsOffset
) internal returns (CallSpec memory) {
) private returns (CallSpec memory) {
CallSpec memory callPolicy;
bool found = false;

Expand Down Expand Up @@ -344,7 +344,7 @@ library SessionLib {
UsageLimit memory limit,
UsageTracker storage tracker,
address account
) internal view returns (uint256) {
) private view returns (uint256) {
if (limit.limitType == LimitType.Unlimited) {
// this might be still limited by `maxValuePerUse` or a constraint
return type(uint256).max;
Expand Down
4 changes: 2 additions & 2 deletions src/managers/HookManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ abstract contract HookManager is IHookManager, SelfAuth {
}
}

function _addHook(address hook, bool isValidation, bytes calldata initData) internal {
function _addHook(address hook, bool isValidation, bytes calldata initData) private {
if (!_supportsHook(hook, isValidation)) {
revert Errors.HOOK_ERC165_FAIL(hook, isValidation);
}
Expand All @@ -114,7 +114,7 @@ abstract contract HookManager is IHookManager, SelfAuth {
emit HookAdded(hook);
}

function _removeHook(address hook, bool isValidation) internal {
function _removeHook(address hook, bool isValidation) private {
if (isValidation) {
require(_validationHooks().remove(hook), "Hook not found");
} else {
Expand Down
3 changes: 2 additions & 1 deletion src/managers/OwnerManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,14 @@ abstract contract OwnerManager is IOwnerManager, SelfAuth {
k1OwnerList = _k1Owners().values();
}

// Should not be set to private as it is called from SsoAccount's initialize
function _k1AddOwner(address addr) internal {
require(_k1Owners().add(addr), "K1 owner already exists");

emit K1OwnerAdded(addr);
}

function _k1RemoveOwner(address addr) internal {
function _k1RemoveOwner(address addr) private {
require(_k1Owners().remove(addr), "K1 owner not found");

emit K1OwnerRemoved(addr);
Expand Down
3 changes: 2 additions & 1 deletion src/managers/ValidatorManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ abstract contract ValidatorManager is IValidatorManager, SelfAuth {
validatorList = _moduleValidators().values();
}

// Should not be set to private as it is called from SsoAccount's initialize
function _addModuleValidator(address validator, bytes memory initData) internal {
if (!_supportsModuleValidator(validator)) {
revert Errors.VALIDATOR_ERC165_FAIL(validator);
Expand All @@ -65,7 +66,7 @@ abstract contract ValidatorManager is IValidatorManager, SelfAuth {
emit ValidatorAdded(validator);
}

function _removeModuleValidator(address validator) internal {
function _removeModuleValidator(address validator) private {
require(_moduleValidators().remove(validator), "Validator not found");

emit ValidatorRemoved(validator);
Expand Down
2 changes: 1 addition & 1 deletion src/validators/SessionKeyValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ contract SessionKeyValidator is IModuleValidator {

/// @notice creates a new session for an account, called by onInstall
/// @param sessionData ABI-encoded session specification
function _addValidationKey(bytes calldata sessionData) internal returns (bool) {
function _addValidationKey(bytes calldata sessionData) private returns (bool) {
SessionLib.SessionSpec memory sessionSpec = abi.decode(sessionData, (SessionLib.SessionSpec));
createSession(sessionSpec);
return true;
Expand Down
2 changes: 1 addition & 1 deletion src/validators/WebAuthValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator {
}

/// @inheritdoc IERC165
function supportsInterface(bytes4 interfaceId) public pure override returns (bool) {
function supportsInterface(bytes4 interfaceId) external pure override returns (bool) {
return
interfaceId == type(IERC165).interfaceId ||
interfaceId == type(IModuleValidator).interfaceId ||
Expand Down