Skip to content

Commit

Permalink
fix: audit N-07
Browse files Browse the repository at this point in the history
  • Loading branch information
jcsec-security committed Feb 9, 2025
1 parent b7ad1df commit c610171
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 12 deletions.
8 changes: 4 additions & 4 deletions src/SsoAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ contract SsoAccount is Initializable, HookManager, ERC1271Handler, TokenCallback
/// @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 @@ -182,7 +182,7 @@ contract SsoAccount is Initializable, HookManager, ERC1271Handler, TokenCallback
/// @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 @@ -212,7 +212,7 @@ contract SsoAccount is Initializable, HookManager, ERC1271Handler, TokenCallback
/// @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 @@ -225,7 +225,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) {
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, Auth {
}
}

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, Auth {
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, Auth {
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, Auth {
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, Auth {
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 @@ -95,7 +95,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

0 comments on commit c610171

Please sign in to comment.