| Number | Details | Instances |
|---|---|---|
| [Medium-1] | Privileged functions can create points of failure | 7 |
| Number | Details | Instances |
|---|---|---|
| [Low-1] | Gas grief possible on unsafe external calls | 1 |
| [Low-2] | Code does not follow the best practice of check-effects-interaction | 1 |
| [Low-3] | Contracts with multiple onlyXYZ modifiers where XYZ is a role can introduce complexities when managing privileges | 1 |
| [Low-4] | Use encodeCall rather than encodeWithSelector | 1 |
| [Low-5] | Solidity version 0.8.20 won't work on all chains due to PUSH0 | 1 |
| [Low-6] | Usage of ecrecover is vulnerable to signature malleability | 1 |
| [Low-7] | Use SafeCast to safely downcast variables | 1 |
| [Low-8] | Uses of EIP712 does not include a salt | 1 |
| [Low-9] | Critical functions should be a two step procedure | 3 |
| [Low-10] | External call recipient may consume all transaction gas | 1 |
| [Low-11] | Critical functions should have a timelock | 3 |
| [Low-12] | Missing contract-existence checks before low-level calls | 3 |
| [Low-13] | Numbers downcast to addresses may result in collisions | 2 |
| [Low-14] | Low Level Calls to Custom Addresses | 3 |
| [Low-15] | Inconsistent use of _msgSender() and msg.sender in contract | 1 |
| [Low-16] | Solidity file is susceptible to .selector-related optimizer bug due to the version used | 4 |
| [Low-17] | Consider a uptime feed on L2 deployments to prevent issues caused by downtime | 5 |
| [Low-18] | Delegate call can fail | 1 |
| Number | Details | Instances |
|---|---|---|
| [NonCritical-1] | Default address(0) can be returned | 3 |
| [NonCritical-2] | Contract existence is not checked before low level call | 1 |
| [NonCritical-3] | Floating pragma should be avoided | 1 |
| [NonCritical-4] | In functions which accept an address as a parameter, there should be a zero address check to prevent bugs | 48 |
| [NonCritical-5] | Enum values should be used in place of constant array indexes | 2 |
| [NonCritical-6] | Default bool values are manually set | 1 |
| [NonCritical-7] | Revert statements within external and public functions can be used to perform DOS attacks | 12 |
| [NonCritical-8] | Private and internal state variables should have a preceding _ in their name unless they are constants | 6 |
| [NonCritical-9] | Contract lines should not be longer than 120 characters for readability | 4 |
| [NonCritical-10] | Not all event definitions are utilizing indexed variables. | 3 |
| [NonCritical-11] | Contracts should have all public/external functions exposed by interfaces | 23 |
| [NonCritical-12] | Functions within contracts are not ordered according to the solidity style guide | 1 |
| [NonCritical-13] | Double type casts create complexity within the code | 2 |
| [NonCritical-14] | Emits without msg.sender parameter | 2 |
| [NonCritical-15] | A function which defines named returns in it's declaration doesn't need to use return | 3 |
| [NonCritical-16] | Unused modifiers present | 2 |
| [NonCritical-17] | Constants should be on the left side of the comparison | 9 |
| [NonCritical-18] | Defined named returns not used within function | 1 |
| [NonCritical-19] | Initialize functions do not emit an event | 1 |
| [NonCritical-20] | Use of non-named numeric constants | 6 |
| [NonCritical-21] | Use immutable not constant for keccak state variables | 3 |
| [NonCritical-22] | Empty bytes check is missing | 14 |
| [NonCritical-23] | Return bool not explicit | 1 |
| [NonCritical-24] | Use OpenZeppelin's ReentrancyGuard/ReentrancyGuardUpgradeable rather than writing your own | 3 |
| [NonCritical-25] | Assembly block creates dirty bits | 1 |
| [NonCritical-26] | call bypasses function existence check, type checking and argument packing | 4 |
| [NonCritical-27] | Cyclomatic complexity in functions | 5 |
| [NonCritical-28] | Contract does not follow the Solidity style guide's suggested layout ordering | 1 |
| [NonCritical-29] | Events may be emitted out of order due to code not follow the best practice of check-effects-interaction | 2 |
| [NonCritical-30] | Missing events in sensitive functions | 7 |
| [NonCritical-31] | Consider implementing EIP-5267 to securely describe EIP-712 domains being used | 1 |
| [NonCritical-32] | Avoid mutating function parameters | 1 |
| [NonCritical-33] | Using an array as a parameter in a external call can be exploited for DOS | 1 |
| [NonCritical-34] | A event should be emitted if a non immutable state variable is set in a constructor | 1 |
| [NonCritical-35] | Use 'using' keyword when using specific imports rather than calling the specific import directly | 1 |
| [NonCritical-36] | Inconsistent checks of address params against address(0) | 1 |
| [NonCritical-37] | Avoid declaring variables with the names of defined functions within the project | 2 |
| [NonCritical-38] | Constructors should emit an event | 3 |
| [NonCritical-39] | Contract and Abstract files should have a fixed compiler version | 5 |
| [NonCritical-40] | Function call in event emit | 1 |
| [NonCritical-41] | Errors should have parameters | 39 |
| [NonCritical-42] | Consider using OpenZeppelins SafeCall library when making calls to arbitrary contracts | 3 |
| [NonCritical-43] | Memory-safe annotation missing | 1 |
| Number | Details | Instances | Gas |
|---|---|---|---|
| [Gas-1] | Avoid updating storage when the value hasn't changed | 1 | 6400 |
| [Gas-2] | State variable can be updated more than once in a function | 3 | 7200 |
| [Gas-3] | Using named returns for pure and view functions is cheaper than using regular returns | 35 | 31850 |
| [Gas-4] | Usage of smaller uint/int types causes overhead | 12 | 7920 |
| [Gas-5] | Use != 0 instead of > 0 | 1 | 3 |
| [Gas-6] | Default int values are manually reset | 2 | 0.0 |
| [Gas-7] | Function calls within for loops | 3 | 0.0 |
| [Gas-8] | For loops in public or external functions should be avoided due to high gas costs and possible DOS | 2 | 0.0 |
| [Gas-9] | Use assembly to check for the zero address | 8 | 0.0 |
| [Gas-10] | Don't use _msgSender() if not supporting EIP-2771 | 3 | 144 |
| [Gas-11] | Use bitmap to save gas | 3 | 630 |
| [Gas-12] | Consider using OZ EnumerateSet in place of nested mappings | 2 | 4000 |
| [Gas-13] | Use selfBalance() in place of address(this).balance | 2 | 3200 |
| [Gas-14] | Use assembly to emit events | 14 | 7448 |
| [Gas-15] | Use assembly in place of abi.decode to extract calldata values more efficiently | 5 | 0.0 |
| [Gas-16] | Using private rather than public for constants and immutables, saves gas | 1 | 0.0 |
| [Gas-17] | Lack of unchecked in loops | 17 | 20400 |
| [Gas-18] | Simple checks for zero uint can be done using assembly to save gas | 6 | 216 |
| [Gas-19] | Using nested if to save gas | 17 | 1734 |
| [Gas-20] | Caching global variables is more expensive than using the actual variable | 1 | 12 |
| [Gas-21] | Low level call can be optimized with assembly | 6 | 8928 |
| [Gas-22] | Remove unused modifiers | 2 | 0.0 |
| [Gas-23] | Variable declared within iteration | 4 | 0.0 |
| [Gas-24] | Internal functions only used once can be inlined to save gas | 15 | 6750 |
| [Gas-25] | Constructors can be marked as payable to save deployment gas | 3 | 0.0 |
| [Gas-26] | Assigning to structs can be more efficient | 5 | 3250 |
| [Gas-27] | Internal functions never used once can be removed | 2 | 0.0 |
| [Gas-28] | It is a waste of GAS to emit variable literals | 4 | 128 |
| [Gas-29] | Short circuit before external calls | 2 | 0.0 |
| [Gas-30] | Use OZ Array.unsafeAccess() to avoid repeated array length checks | 11 | 254100 |
| [Gas-31] | State variable written in a loop | 2 | 23176 |
| [Gas-32] | Use uint256(1)/uint256(2) instead of true/false to save gas for changes | 3 | 153000 |
| [Gas-33] | Call msg.XYZ directly rather than caching the value to save gas | 1 | 0.0 |
| [Gas-34] | Consider pre-calculating the address of address(this) to save gas | 14 | 0.0 |
| [Gas-35] | Use constants instead of type(uint).max | 2 | 0.0 |
Ensure such accounts are protected and consider implementing multi sig to prevent a single point of failure
Num of instances: 7
Click to show findings
['292']
292: function setLockdownMode(bytes19 addressPrefix, bool enabled) public payable virtual onlyOwner(addressPrefix) // <= FOUND['307']
307: function setPermitDisabledMode(
308: bytes19 addressPrefix,
309: bool enabled
310: ) public payable virtual onlyOwner(addressPrefix) // <= FOUND['325']
325: function setNonce(
326: bytes19 addressPrefix,
327: uint256 nonceNamespace,
328: uint256 nonce
329: ) public payable virtual onlyOwner(addressPrefix) // <= FOUND['418']
418: function enableCollateral(
419: address account,
420: address vault
421: ) public payable virtual nonReentrantChecksAndControlCollateral onlyOwnerOrOperator(account) // <= FOUND['431']
431: function disableCollateral(
432: address account,
433: address vault
434: ) public payable virtual nonReentrantChecksAndControlCollateral onlyOwnerOrOperator(account) // <= FOUND['442']
442: function reorderCollaterals(
443: address account,
444: uint8 index1,
445: uint8 index2
446: ) public payable virtual nonReentrantChecksAndControlCollateral onlyOwnerOrOperator(account) // <= FOUND['464']
464: function enableController(
465: address account,
466: address vault
467: ) public payable virtual nonReentrantChecksAndControlCollateral onlyOwnerOrOperator(account) // <= FOUNDIn Solidity, the use of low-level call methods can expose contracts to gas griefing attacks. The potential problem arises when the callee contract returns a large amount of data. This data is allocated in the memory of the calling contract, which pays for the gas costs. If the callee contract intentionally returns an enormous amount of data, the gas costs can skyrocket, causing the transaction to fail due to an Out of Gas error. Therefore, it's advisable to limit the use of call when interacting with untrusted contracts, or ensure that the callee's returned data size is capped or known in advance to prevent unexpected high gas costs.
Num of instances: 1
Click to show findings
['653']
653: function batchSimulation(BatchItem[] calldata items) // <= FOUND
654: external
655: payable
656: virtual
657: returns (
658: BatchItemResult[] memory batchItemsResult,
659: StatusCheckResult[] memory accountsStatusCheckResult,
660: StatusCheckResult[] memory vaultsStatusCheckResult
661: )
662: {
663: (bool success, bytes memory result) = address(this).delegatecall(abi.encodeCall(this.batchRevert, items));
664:
665: if (success) {
666: revert EVC_BatchPanic();
667: } else if (result.length < 4 || bytes4(result) != EVC_RevertedBatchResult.selector) {
668: revertBytes(result);
669: }
670:
671: assembly {
672: let length := mload(result)
673:
674: result := add(result, 4)
675:
676:
677: mstore(result, sub(length, 4))
678: }
679:
680: (batchItemsResult, accountsStatusCheckResult, vaultsStatusCheckResult) =
681: abi.decode(result, (BatchItemResult[], StatusCheckResult[], StatusCheckResult[]));
682: }The "check-effects-interaction" pattern is a best practice in smart contract development, emphasizing the order of operations in functions to prevent reentrancy attacks. Violations arise when a function interacts with external contracts before settling internal state changes or checks. This misordering can expose the contract to potential threats. To adhere to this pattern, first ensure all conditions or checks are satisfied, then update any internal states, and only after these steps, interact with external contracts or addresses. Rearranging operations to this recommended sequence bolsters contract security and aligns with established best practices in the Ethereum community.
Num of instances: 1
Click to show findings
['930']
930: function checkAccountStatusInternal(address account) internal virtual returns (bool isValid, bytes memory result) { // <= FOUND
931: SetStorage storage accountControllersStorage = accountControllers[account];
932: uint256 numOfControllers = accountControllersStorage.numElements;
933: address controller = accountControllersStorage.firstElement;
934: uint8 stamp = accountControllersStorage.stamp;
935:
936: if (numOfControllers == 0) return (true, "");
937: else if (numOfControllers > 1) return (false, abi.encodeWithSelector(EVC_ControllerViolation.selector));
938:
939: bool success;
940: (success, result) = // <= FOUND
941: controller.call(abi.encodeCall(IVault.checkAccountStatus, (account, accountCollaterals[account].get())));
942:
943: isValid = success && result.length == 32
944: && abi.decode(result, (bytes32)) == bytes32(IVault.checkAccountStatus.selector);
945:
946: if (isValid) {
947: accountControllersStorage.numElements = uint8(numOfControllers);
948: accountControllersStorage.firstElement = controller;
949: accountControllersStorage.metadata = uint80(block.timestamp);
950: accountControllersStorage.stamp = stamp;
951: }
952:
953: emit AccountStatusCheck(account, controller);
954: }[Low-3] Contracts with multiple onlyXYZ modifiers where XYZ is a role can introduce complexities when managing privileges
In smart contracts, using multiple onlyXYZ modifiers for different roles can complicate privilege management. OpenZeppelin's AccessControl offers a streamlined solution, enabling easier and more flexible role-based permission handling. It simplifies the assignment and revocation of roles compared to multiple individual modifiers, reducing potential errors and improving contract maintainability. This modular approach to access management makes it more straightforward to define, manage, and audit roles and permissions within a contract.
Num of instances: 1
Click to show findings
['18']
18: contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC {
19: using ExecutionContext for EC;
20: using Set for SetStorage;
115: modifier onlyOwner(bytes19 addressPrefix) { // <= FOUND
116: authenticateCaller({addressPrefix: addressPrefix, allowOperator: false, checkLockdownMode: false});
117:
118: _;
119: }
120:
127: modifier onlyOwnerOrOperator(address account) { // <= FOUND
127: modifier onlyOwnerOrOperator(address account) { // <= FOUND
128: authenticateCaller({account: account, allowOperator: true, checkLockdownMode: true});
129:
130: _;
131: }
132:
135: modifier onlyController(address account) { // <= FOUND
135: modifier onlyController(address account) { // <= FOUND
136: {
137: uint256 numOfControllers = accountControllers[account].numElements;
138: address controller = accountControllers[account].firstElement;
139:
140: if (numOfControllers != 1) {
141: revert EVC_ControllerViolation();
Unlike encodeCall, encodeWithSelector does not perform checks to verify the params passed are compatible with a call, as such it is safer to use encodeCall to prevent potential issues.
Num of instances: 1
Click to show findings
['937']
937: else if (numOfControllers > 1) return (false, abi.encodeWithSelector(EVC_ControllerViolation.selector)); // <= FOUNDSolidity version 0.8.20 uses the new Shanghai EVM which introduces the PUSH0 opcode, this may not be implemented on all chains and L2 thus reducing the portability and compatability of the code. Consider using a earlier solidity version.
Num of instances: 1
The ecrecover function in Ethereum smart contracts is vulnerable to signature malleability due to the nature of Ethereum signatures and the ECDSA (Elliptic Curve Digital Signature Algorithm) used. Ethereum signatures include an additional field, v (the recovery identifier), which can have multiple valid values for the same signer and message. This field, which ranges from 27 to 30, helps to determine the correct public key associated with a signature. The vulnerability arises because there are multiple valid s values for any given r in a signature, meaning (r, s) and (r, n - s) can both be valid for the same message and signing key. The ecrecover function, which is used to verify signatures and recover addresses, must account for these different valid s values, leading to potential signature malleability issues.
Num of instances: 1
Downcasting int/uints in Solidity can be unsafe due to the potential for data loss and unintended behavior. When downcasting a larger integer type to a smaller one (e.g., uint256 to uint128), the value may exceed the range of the target type, leading to truncation and loss of significant digits. This data loss can result in unexpected state changes, incorrect calculations, or other contract vulnerabilities, ultimately compromising the contracts functionality and reliability. To prevent these risks, developers should carefully consider the range of values their variables may hold and ensure that proper checks are in place to prevent out-of-range values before performing downcasting. Also consider using OZ SafeCast functionality.
Num of instances: 1
Click to show findings
['110']
110: function remove(SetStorage storage setStorage, address element) internal returns (bool) { // <= FOUND
111: address firstElement = setStorage.firstElement;
112: uint256 numElements = setStorage.numElements;
113: uint80 metadata = setStorage.metadata;
114:
115: if (numElements == 0) return false;
116:
117: uint256 searchIndex;
118: if (firstElement != element) {
119: for (searchIndex = EMPTY_ELEMENT_OFFSET; searchIndex < numElements; ++searchIndex) {
120: if (setStorage.elements[searchIndex].value == element) break;
121: }
122:
123: if (searchIndex == numElements) return false;
124: }
125:
126:
127: if (numElements == 1) {
128: setStorage.numElements = 0;
129: setStorage.firstElement = address(0);
130: setStorage.metadata = metadata;
131: setStorage.stamp = DUMMY_STAMP;
132: return true;
133: }
134:
135: uint256 lastIndex;
136: unchecked {
137: lastIndex = numElements - 1;
138: }
139:
140:
141:
142: ElementStorage storage lastElement = setStorage.elements[lastIndex];
143: if (searchIndex != lastIndex) {
144: if (searchIndex == 0) {
145: setStorage.firstElement = lastElement.value;
146: setStorage.numElements = uint8(lastIndex); // <= FOUND
147: setStorage.metadata = metadata;
148: setStorage.stamp = DUMMY_STAMP;
149: } else {
150: setStorage.elements[searchIndex].value = lastElement.value;
151:
152: setStorage.firstElement = firstElement;
153: setStorage.numElements = uint8(lastIndex); // <= FOUND
154: setStorage.metadata = metadata;
155: setStorage.stamp = DUMMY_STAMP;
156: }
157: } else {
158: setStorage.firstElement = firstElement;
159: setStorage.numElements = uint8(lastIndex);
160: setStorage.metadata = metadata;
161: setStorage.stamp = DUMMY_STAMP;
162: }
163:
164: lastElement.value = address(0);
165:
166: return true;
167: }It is standard for uses of EIP712 to include a salt, not doing so can cause future incompatibilities and in this instance cause hash collisions do to no salting
Num of instances: 1
Click to show findings
['36']
36: bytes32 internal constant TYPE_HASH =
37: keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); // <= FOUNDCritical functions in Solidity contracts should follow a two-step procedure to enhance security, minimize human error, and ensure proper access control. By dividing sensitive operations into distinct phases, such as initiation and confirmation, developers can introduce a safeguard against unintended actions or unauthorized access.
Num of instances: 3
Click to show findings
['292']
292: function setLockdownMode(bytes19 addressPrefix, bool enabled) public payable virtual onlyOwner(addressPrefix) // <= FOUND['307']
307: function setPermitDisabledMode( // <= FOUND
308: bytes19 addressPrefix,
309: bool enabled
310: ) public payable virtual onlyOwner(addressPrefix) ['325']
325: function setNonce( // <= FOUND
326: bytes19 addressPrefix,
327: uint256 nonceNamespace,
328: uint256 nonce
329: ) public payable virtual onlyOwner(addressPrefix) When making external calls, the called contract can intentionally or unintentionally consume all provided gas, leading to unintended transaction reversion. To mitigate this risk, it's crucial to specify a gas limit when making the call. By using addr.call{gas: <amount>}(""), you allocate a specific amount of gas to the external call, ensuring the parent transaction has gas left for post-call operations. This approach safeguards against malevolent contracts aiming to exhaust gas and provides greater control over transaction execution.
Num of instances: 1
Click to show findings
['862']
862: (success, result) = targetContract.call{value: value}(data); // <= FOUNDCritical functions, especially those affecting protocol parameters or user funds, are potential points of failure or exploitation. To mitigate risks, incorporating a timelock on such functions can be beneficial. A timelock requires a waiting period between the time an action is initiated and when it's executed, giving stakeholders time to react, potentially vetoing malicious or erroneous changes. To implement, integrate a smart contract like OpenZeppelin's TimelockController or build a custom mechanism. This ensures governance decisions or administrative changes are transparent and allows for community or multi-signature interventions, enhancing protocol security and trustworthiness.
Num of instances: 3
Click to show findings
['292']
292: function setLockdownMode(bytes19 addressPrefix, bool enabled) public payable virtual onlyOwner(addressPrefix) // <= FOUND['307']
307: function setPermitDisabledMode( // <= FOUND
308: bytes19 addressPrefix,
309: bool enabled
310: ) public payable virtual onlyOwner(addressPrefix) ['325']
325: function setNonce( // <= FOUND
326: bytes19 addressPrefix,
327: uint256 nonceNamespace,
328: uint256 nonce
329: ) public payable virtual onlyOwner(addressPrefix) Low-level calls in Solidity, when made to addresses without contract code, don't fail but return a successful status. This behavior can be misleading, leading to unintended consequences in dApps. Ignoring this can potentially mean acting on false positive results. To address this, apart from the conventional zero-address check, developers should verify the existence of contract code at the target address by ensuring that the code length at the specified address (<address>.code.length) is greater than zero. By doing so, it provides a more robust validation before executing low-level calls, safeguarding against unintentional interactions with empty addresses.
Num of instances: 3
Click to show findings
['930']
930: function checkAccountStatusInternal(address account) internal virtual returns (bool isValid, bytes memory result) {
931: SetStorage storage accountControllersStorage = accountControllers[account];
932: uint256 numOfControllers = accountControllersStorage.numElements;
933: address controller = accountControllersStorage.firstElement;
934: uint8 stamp = accountControllersStorage.stamp;
935:
936: if (numOfControllers == 0) return (true, "");
937: else if (numOfControllers > 1) return (false, abi.encodeWithSelector(EVC_ControllerViolation.selector));
938:
939: bool success;
940: (success, result) =
941: controller.call(abi.encodeCall(IVault.checkAccountStatus, (account, accountCollaterals[account].get()))); // <= FOUND
942:
943: isValid = success && result.length == 32
944: && abi.decode(result, (bytes32)) == bytes32(IVault.checkAccountStatus.selector);
945:
946: if (isValid) {
947: accountControllersStorage.numElements = uint8(numOfControllers);
948: accountControllersStorage.firstElement = controller;
949: accountControllersStorage.metadata = uint80(block.timestamp);
950: accountControllersStorage.stamp = stamp;
951: }
952:
953: emit AccountStatusCheck(account, controller);
954: }['977']
977: function checkVaultStatusInternal(address vault) internal returns (bool isValid, bytes memory result) {
978: bool success;
979: (success, result) = vault.call(abi.encodeCall(IVault.checkVaultStatus, ())); // <= FOUND
980:
981: isValid =
982: success && result.length == 32 && abi.decode(result, (bytes32)) == bytes32(IVault.checkVaultStatus.selector);
983:
984: emit VaultStatusCheck(vault);
985: }['827']
827: function callWithContextInternal(
828: address targetContract,
829: address onBehalfOfAccount,
830: uint256 value,
831: bytes calldata data
832: ) internal virtual returns (bool success, bytes memory result) {
833: if (value == type(uint256).max) {
834: value = address(this).balance;
835: } else if (value > address(this).balance) {
836: revert EVC_InvalidValue();
837: }
838:
839: EC contextCache = executionContext;
840: address msgSender = _msgSender();
841:
842:
843:
844:
845:
846:
847:
848:
849: if (
850: haveCommonOwnerInternal(onBehalfOfAccount, msgSender) || targetContract == msg.sender
851: || targetContract == address(this) || contextCache.isControlCollateralInProgress()
852: ) {
853: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).clearOperatorAuthenticated();
854: } else {
855: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).setOperatorAuthenticated();
856: }
857:
858: emit CallWithContext(
859: msgSender, getAddressPrefixInternal(onBehalfOfAccount), onBehalfOfAccount, targetContract, bytes4(data)
860: );
861:
862: (success, result) = targetContract.call{value: value}(data); // <= FOUND
863:
864: executionContext = contextCache;
865: }Downcasting numbers to addresses in blockchain contracts, particularly in Ethereum's Solidity, involves risks such as possible address collisions. A collision occurs when different inputs, when cast or hashed, generate the same output address, potentially compromising contract integrity and asset security. If an uint256, for instance, is downcast to an address (effectively an uint160) without ensuring it’s a legitimate, collision-free conversion, different uint256 inputs might yield the same address, creating vulnerabilities attackers might exploit. Implementing thorough checks and opting for secure practices, like avoiding downcasting in critical logic or utilizing mappings with original uint256 as keys, mitigates risks
Num of instances: 2
Click to show findings
['812']
812: address phantomAccount = address(uint160(uint152(addressPrefix)) << ACCOUNT_ID_OFFSET); // <= FOUND['40']
40: result = address(uint160(EC.unwrap(self) & ON_BEHALF_OF_ACCOUNT_MASK)); // <= FOUNDLow-level calls (such as .call(), .delegatecall(), or .callcode()) in Solidity provide a way to interact with other contracts or addresses. However, when these calls are made to addresses that are provided as parameters or are not well-validated, they pose a significant security risk. Untrusted addresses might contain malicious code leading to unexpected behavior, loss of funds, or vulnerabilities.
Resolution: Prefer using high-level Solidity function calls or interface-based interactions with known contracts to ensure security. If low-level calls are necessary, rigorously validate the addresses and test all possible interactions. Implementing additional checks and fail-safes can help mitigate potential risks associated with low-level calls.
Num of instances: 3
Click to show findings
['827']
827: function callWithContextInternal(
828: address targetContract,
829: address onBehalfOfAccount,
830: uint256 value,
831: bytes calldata data
832: ) internal virtual returns (bool success, bytes memory result) {
833: if (value == type(uint256).max) {
834: value = address(this).balance;
835: } else if (value > address(this).balance) {
836: revert EVC_InvalidValue();
837: }
838:
839: EC contextCache = executionContext;
840: address msgSender = _msgSender();
841:
842:
843:
844:
845:
846:
847:
848:
849: if (
850: haveCommonOwnerInternal(onBehalfOfAccount, msgSender) || targetContract == msg.sender
851: || targetContract == address(this) || contextCache.isControlCollateralInProgress()
852: ) {
853: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).clearOperatorAuthenticated();
854: } else {
855: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).setOperatorAuthenticated();
856: }
857:
858: emit CallWithContext(
859: msgSender, getAddressPrefixInternal(onBehalfOfAccount), onBehalfOfAccount, targetContract, bytes4(data)
860: );
861:
862: (success, result) = targetContract.call{value: value}(data); // <= FOUND
863:
864: executionContext = contextCache;
865: }['930']
930: function checkAccountStatusInternal(address account) internal virtual returns (bool isValid, bytes memory result) { // <= FOUND
931: SetStorage storage accountControllersStorage = accountControllers[account];
932: uint256 numOfControllers = accountControllersStorage.numElements;
933: address controller = accountControllersStorage.firstElement;
934: uint8 stamp = accountControllersStorage.stamp;
935:
936: if (numOfControllers == 0) return (true, "");
937: else if (numOfControllers > 1) return (false, abi.encodeWithSelector(EVC_ControllerViolation.selector));
938:
939: bool success;
940: (success, result) =
941: controller.call(abi.encodeCall(IVault.checkAccountStatus, (account, accountCollaterals[account].get()))); // <= FOUND
942:
943: isValid = success && result.length == 32
944: && abi.decode(result, (bytes32)) == bytes32(IVault.checkAccountStatus.selector);
945:
946: if (isValid) {
947: accountControllersStorage.numElements = uint8(numOfControllers);
948: accountControllersStorage.firstElement = controller;
949: accountControllersStorage.metadata = uint80(block.timestamp);
950: accountControllersStorage.stamp = stamp;
951: }
952:
953: emit AccountStatusCheck(account, controller);
954: }['977']
977: function checkVaultStatusInternal(address vault) internal returns (bool isValid, bytes memory result) { // <= FOUND
978: bool success;
979: (success, result) = vault.call(abi.encodeCall(IVault.checkVaultStatus, ())); // <= FOUND
980:
981: isValid =
982: success && result.length == 32 && abi.decode(result, (bytes32)) == bytes32(IVault.checkVaultStatus.selector);
983:
984: emit VaultStatusCheck(vault);
985: }For the sake of consistency, stick to using only one of these values throughout the contract. Not doing so in this case can be quite harmful as _msgSender and msg.sender do have some differences, one being that msgSender cannot be used to determine if an account is a EOA but msg.sender can. Differences like these can introduce vulnerabilities is they are not properly acknowledged by the dev team.
Num of instances: 1
Click to show findings
['18']
18: contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC {
19: using ExecutionContext for EC;
144: if (controller != msg.sender) { // <= FOUND
478: if (accountControllers[account].remove(msg.sender)) { // <= FOUND
479: emit ControllerStatus(account, msg.sender, false); // <= FOUND
499: if (inPermitSelfCall() || (sender != address(0) && sender != msg.sender)) { // <= FOUND
726: vaultStatusChecks.insert(msg.sender); // <= FOUND
728: requireVaultStatusCheckInternalNonReentrantChecks(msg.sender); // <= FOUND
734: vaultStatusChecks.remove(msg.sender); // <= FOUND
741: vaultStatusChecks.insert(msg.sender); // <= FOUND
744: requireVaultStatusCheckInternalNonReentrantChecks(msg.sender); // <= FOUND
767: address msgSender = _msgSender(); // <= FOUND
840: address msgSender = _msgSender(); // <= FOUND
850: haveCommonOwnerInternal(onBehalfOfAccount, msgSender) || targetContract == msg.sender // <= FOUND
897: if (targetContract != msg.sender) { // <= FOUND
1160: function _msgSender() internal view virtual returns (address) { // <= FOUND
1161: return inPermitSelfCall() ? executionContext.getOnBehalfOfAccount() : msg.sender; // <= FOUND
1168: return address(this) == msg.sender; // <= FOUND
In Solidity versions prior to 0.8.21, a bug related to the .selector usage in optimizer settings could lead to incorrect code generation, where function evaluations using .selector might not execute as intended. This issue is classified as low-severity, mainly affecting uncommon code patterns where a function call is used instead of a direct contract name for selector lookup. Files using .selector with affected Solidity versions should be reviewed and updated to mitigate potential issues, ensuring contracts function correctly and securely.
Num of instances: 4
Click to show findings
['653']
653: function batchSimulation(BatchItem[] calldata items)
654: external
655: payable
656: virtual
657: returns (
658: BatchItemResult[] memory batchItemsResult,
659: StatusCheckResult[] memory accountsStatusCheckResult,
660: StatusCheckResult[] memory vaultsStatusCheckResult
661: )
662: {
663: (bool success, bytes memory result) = address(this).delegatecall(abi.encodeCall(this.batchRevert, items));
664:
665: if (success) {
666: revert EVC_BatchPanic();
667: } else if (result.length < 4 || bytes4(result) != EVC_RevertedBatchResult.selector) { // <= FOUND
668: revertBytes(result);
669: }
670:
671: assembly {
672: let length := mload(result)
673:
674: result := add(result, 4)
675:
676:
677: mstore(result, sub(length, 4))
678: }
679:
680: (batchItemsResult, accountsStatusCheckResult, vaultsStatusCheckResult) =
681: abi.decode(result, (BatchItemResult[], StatusCheckResult[], StatusCheckResult[]));
682: }['930']
930: function checkAccountStatusInternal(address account) internal virtual returns (bool isValid, bytes memory result) {
931: SetStorage storage accountControllersStorage = accountControllers[account];
932: uint256 numOfControllers = accountControllersStorage.numElements;
933: address controller = accountControllersStorage.firstElement;
934: uint8 stamp = accountControllersStorage.stamp;
935:
936: if (numOfControllers == 0) return (true, "");
937: else if (numOfControllers > 1) return (false, abi.encodeWithSelector(EVC_ControllerViolation.selector)); // <= FOUND
938:
939: bool success;
940: (success, result) =
941: controller.call(abi.encodeCall(IVault.checkAccountStatus, (account, accountCollaterals[account].get())));
942:
943: isValid = success && result.length == 32
944: && abi.decode(result, (bytes32)) == bytes32(IVault.checkAccountStatus.selector); // <= FOUND
945:
946: if (isValid) {
947: accountControllersStorage.numElements = uint8(numOfControllers);
948: accountControllersStorage.firstElement = controller;
949: accountControllersStorage.metadata = uint80(block.timestamp);
950: accountControllersStorage.stamp = stamp;
951: }
952:
953: emit AccountStatusCheck(account, controller);
954: }['977']
977: function checkVaultStatusInternal(address vault) internal returns (bool isValid, bytes memory result) {
978: bool success;
979: (success, result) = vault.call(abi.encodeCall(IVault.checkVaultStatus, ()));
980:
981: isValid =
982: success && result.length == 32 && abi.decode(result, (bytes32)) == bytes32(IVault.checkVaultStatus.selector); // <= FOUND
983:
984: emit VaultStatusCheck(vault);
985: }['1134']
1134: function isValidERC1271Signature(
1135: address signer,
1136: bytes32 hash,
1137: bytes memory signature
1138: ) internal view returns (bool isValid) {
1139: if (signer.code.length == 0) return false;
1140:
1141: (bool success, bytes memory result) =
1142: signer.staticcall(abi.encodeCall(IERC1271.isValidSignature, (hash, signature)));
1143:
1144: isValid = success && result.length == 32
1145: && abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector); // <= FOUND
1146: }In L2 deployments, incorporating an uptime feed is crucial to mitigate issues arising from sequencer downtime. Downtime can disrupt services, leading to transaction failures or incorrect data readings, affecting overall system reliability. By integrating an uptime feed, you gain insight into the operational status of the L2 network, enabling proactive measures like halting sensitive operations or alerting users. This approach ensures that your contract behaves predictably and securely during network outages, enhancing the robustness and reliability of your decentralized application, which is especially important in mission-critical or high-stakes environments.
Num of instances: 5
Click to show findings
['11']
11: contract Errors // <= FOUND['18']
18: contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC // <= FOUND['9']
9: contract Events // <= FOUND['13']
13: abstract contract EVCUtil // <= FOUND['14']
14: abstract contract TransientStorage // <= FOUNDWhen a non payable function contains a payable call, it is important to point up that the delegate call will be limited to only calling other non payable functions. This can potentially cause future issues if this is not accounted for.
Num of instances: 1
Click to show findings
['877']
877: function callWithAuthenticationInternal(
878: address targetContract,
879: address onBehalfOfAccount,
880: uint256 value,
881: bytes calldata data
882: ) internal virtual returns (bool success, bytes memory result) {
883: if (targetContract == address(this)) {
884: if (onBehalfOfAccount != address(0)) {
885: revert EVC_InvalidAddress();
886: }
887:
888: if (value != 0) {
889: revert EVC_InvalidValue();
890: }
891:
892:
893: (success, result) = address(this).delegatecall(data); // <= FOUND
894: } else {
895:
896:
897: if (targetContract != msg.sender) {
898: authenticateCaller({account: onBehalfOfAccount, allowOperator: true, checkLockdownMode: true});
899: }
900:
901: (success, result) = callWithContextInternal(targetContract, onBehalfOfAccount, value, data);
902: }
903: }Allowing a function in Solidity to return the default address (address(0)) can be problematic as it can represent uninitialized or invalid addresses. If such an address is utilized in transfer operations or other sensitive actions, it could lead to loss of funds or unpredicted behavior. It's prudent to include checks in your functions to prevent the return of the zero address, enhancing contract security.
Num of instances: 3
Click to show findings
['28']
28: function EVC() external view returns (address) {
29: return address(evc);
30: }['262']
262: function getAccountOwner(address account) external view returns (address) {
263: return getAccountOwnerInternal(account);
264: }['1193']
1193: function getAccountOwnerInternal(address account) internal view returns (address) {
1194: bytes19 addressPrefix = getAddressPrefixInternal(account);
1195: return ownerLookup[addressPrefix].owner;
1196: }Low-level assembly calls, such as call, can return a successful status even when the target address contains no executable code. This is due to the fact that a call operation merely checks if the call operation itself was successful, not whether the call was made to an address with code. As a result, these calls might lead to false positives when assessing the success of an operation. To prevent potential issues, consider using Solidity's extcodesize function to check the size of the contract code at the target address. If extcodesize returns zero, this indicates that there is no code at the specified address, and the function can safely revert.
Num of instances: 1
Click to show findings
['42']
42: assembly {
43: mstore(0, 0x1f8b521500000000000000000000000000000000000000000000000000000000)
44: mstore(4, address())
45: mstore(36, caller())
46: mstore(68, callvalue())
47: mstore(100, 128)
48: mstore(132, calldatasize())
49: calldatacopy(164, 0, calldatasize())
50:
51:
52:
53: mstore(add(164, calldatasize()), 0)
54: let result := call(gas(), _evc, callvalue(), 0, add(164, and(add(calldatasize(), 31), not(31))), 0, 0) // <= FOUND
55:
56: returndatacopy(0, 0, returndatasize())
57: switch result
58: case 0 { revert(0, returndatasize()) }
59: default { return(64, sub(returndatasize(), 64)) }
60: }Num of instances: 1
[NonCritical-4] In functions which accept an address as a parameter, there should be a zero address check to prevent bugs
In smart contract development, especially with Solidity, it's crucial to validate inputs to functions. When a function accepts an Ethereum address as a parameter, implementing a zero address check (i.e., ensuring the address is not 0x0) is a best practice to prevent potential bugs and vulnerabilities. The zero address (0x0) is a default value and generally indicates an uninitialized or invalid state. Passing the zero address to certain functions can lead to unintended behaviors, like funds getting locked permanently or transactions failing silently. By checking for and rejecting the zero address, developers can ensure that the function operates as intended and interacts only with valid Ethereum addresses. This check enhances the contract's robustness and security.
Num of instances: 48
Click to show findings
['252']
252: function haveCommonOwner(address account, address otherAccount) external pure returns (bool) ['257']
257: function getAddressPrefix(address account) external pure returns (bytes19) ['262']
262: function getAccountOwner(address account) external view returns (address) ['267']
267: function isLockdownMode(bytes19 addressPrefix) external view returns (bool) ['272']
272: function isPermitDisabledMode(bytes19 addressPrefix) external view returns (bool) ['277']
277: function getNonce(bytes19 addressPrefix, uint256 nonceNamespace) external view returns (uint256) ['282']
282: function getOperator(bytes19 addressPrefix, address operator) external view returns (uint256) ['287']
287: function isAccountOperatorAuthorized(address account, address operator) external view returns (bool) ['292']
292: function setLockdownMode(bytes19 addressPrefix, bool enabled) public payable virtual onlyOwner(addressPrefix) ['307']
307: function setPermitDisabledMode(
308: bytes19 addressPrefix,
309: bool enabled
310: ) public payable virtual onlyOwner(addressPrefix) ['325']
325: function setNonce(
326: bytes19 addressPrefix,
327: uint256 nonceNamespace,
328: uint256 nonce
329: ) public payable virtual onlyOwner(addressPrefix) ['344']
344: function setOperator(bytes19 addressPrefix, address operator, uint256 operatorBitField) public payable virtual ['365']
365: function setAccountOperator(address account, address operator, bool authorized) public payable virtual ['408']
408: function getCollaterals(address account) external view returns (address[] memory) ['413']
413: function isCollateralEnabled(address account, address vault) external view returns (bool) ['418']
418: function enableCollateral(
419: address account,
420: address vault
421: ) public payable virtual nonReentrantChecksAndControlCollateral onlyOwnerOrOperator(account) ['431']
431: function disableCollateral(
432: address account,
433: address vault
434: ) public payable virtual nonReentrantChecksAndControlCollateral onlyOwnerOrOperator(account) ['442']
442: function reorderCollaterals(
443: address account,
444: uint8 index1,
445: uint8 index2
446: ) public payable virtual nonReentrantChecksAndControlCollateral onlyOwnerOrOperator(account) ['454']
454: function getControllers(address account) external view returns (address[] memory) ['459']
459: function isControllerEnabled(address account, address vault) external view returns (bool) ['464']
464: function enableController(
465: address account,
466: address vault
467: ) public payable virtual nonReentrantChecksAndControlCollateral onlyOwnerOrOperator(account) ['477']
477: function disableController(address account) public payable virtual nonReentrantChecksAndControlCollateral ['554']
554: function call(
555: address targetContract,
556: address onBehalfOfAccount,
557: uint256 value,
558: bytes calldata data
559: ) public payable virtual nonReentrantChecksAndControlCollateral returns (bytes memory result) ['572']
572: function controlCollateral(
573: address targetCollateral,
574: address onBehalfOfAccount,
575: uint256 value,
576: bytes calldata data
577: )
578: public
579: payable
580: virtual
581: nonReentrantChecksAndControlCollateral
582: onlyController(onBehalfOfAccount)
583: returns (bytes memory result)
584: ['687']
687: function getLastAccountStatusCheckTimestamp(address account) external view nonReentrantChecks returns (uint256) ['692']
692: function isAccountStatusCheckDeferred(address account) external view nonReentrantChecks returns (bool) ['697']
697: function requireAccountStatusCheck(address account) public payable virtual ['706']
706: function forgiveAccountStatusCheck(address account)
707: public
708: payable
709: virtual
710: nonReentrantChecksAcquireLock
711: onlyController(account)
712: ['719']
719: function isVaultStatusCheckDeferred(address vault) external view nonReentrantChecks returns (bool) ['738']
738: function requireAccountAndVaultStatusCheck(address account) public payable virtual ['807']
807: function authenticateCaller(
808: bytes19 addressPrefix,
809: bool allowOperator,
810: bool checkLockdownMode
811: ) internal virtual returns (address) ['827']
827: function callWithContextInternal(
828: address targetContract,
829: address onBehalfOfAccount,
830: uint256 value,
831: bytes calldata data
832: ) internal virtual returns (bool success, bytes memory result) ['930']
930: function checkAccountStatusInternal(address account) internal virtual returns (bool isValid, bytes memory result) ['956']
956: function requireAccountStatusCheckInternal(address account) internal virtual ['964']
964: function requireAccountStatusCheckInternalNonReentrantChecks(address account)
965: internal
966: virtual
967: nonReentrantChecksAcquireLock
968: ['977']
977: function checkVaultStatusInternal(address vault) internal returns (bool isValid, bytes memory result) ['987']
987: function requireVaultStatusCheckInternal(address vault) internal virtual ['995']
995: function requireVaultStatusCheckInternalNonReentrantChecks(address vault)
996: internal
997: virtual
998: nonReentrantChecksAcquireLock
999: ['1041']
1041: function isSignerValid(address signer) internal pure virtual returns (bool) ['1056']
1056: function getPermitHash(
1057: address signer,
1058: address sender,
1059: uint256 nonceNamespace,
1060: uint256 nonce,
1061: uint256 deadline,
1062: uint256 value,
1063: bytes calldata data
1064: ) internal view returns (bytes32 permitHash) ['1134']
1134: function isValidERC1271Signature(
1135: address signer,
1136: bytes32 hash,
1137: bytes memory signature
1138: ) internal view returns (bool isValid) ['1175']
1175: function haveCommonOwnerInternal(address account, address otherAccount) internal pure returns (bool result) ['1186']
1186: function getAddressPrefixInternal(address account) internal pure returns (bytes19) ['1193']
1193: function getAccountOwnerInternal(address account) internal view returns (address) ['43']
43: function setOnBehalfOfAccount(EC self, address account) internal pure returns (EC result) ['71']
71: function insert(SetStorage storage setStorage, address element) internal returns (bool) ['110']
110: function remove(SetStorage storage setStorage, address element) internal returns (bool) ['232']
232: function contains(SetStorage storage setStorage, address element) internal view returns (bool) Create a commented enum value to use in place of constant array indexes, this makes the code far easier to understand
Num of instances: 2
Click to show findings
['211']
211: output[0] = firstElement; // <= FOUND['298']
298: results[0] = abi.encode(firstElement, success, result); // <= FOUNDIn instances where a new variable is defined, there is no need to set it to it's default value.
Num of instances: 1
[NonCritical-7] Revert statements within external and public functions can be used to perform DOS attacks
In Solidity, 'revert' statements are used to undo changes and throw an exception when certain conditions are not met. However, in public and external functions, improper use of revert can be exploited for Denial of Service (DoS) attacks. An attacker can intentionally trigger these 'revert' conditions, causing legitimate transactions to consistently fail. For example, if a function relies on specific conditions from user input or contract state, an attacker could manipulate these to continually force reverts, blocking the function's execution. Therefore, it's crucial to design contract logic to handle exceptions properly and avoid scenarios where revert can be predictably triggered by malicious actors. This includes careful input validation and considering alternative design patterns that are less susceptible to such abuses.
Num of instances: 12
Click to show findings
['208']
208: function getCurrentOnBehalfOfAccount(address controllerToCheck)
209: external
210: view
211: returns (address onBehalfOfAccount, bool controllerEnabled)
212: {
213: onBehalfOfAccount = executionContext.getOnBehalfOfAccount();
214:
215:
216: if (onBehalfOfAccount == address(0)) {
217: revert EVC_OnBehalfOfAccountNotAuthenticated(); // <= FOUND
218: }
219:
220: controllerEnabled =
221: controllerToCheck == address(0) ? false : accountControllers[onBehalfOfAccount].contains(controllerToCheck);
222: }['653']
653: function batchSimulation(BatchItem[] calldata items)
654: external
655: payable
656: virtual
657: returns (
658: BatchItemResult[] memory batchItemsResult,
659: StatusCheckResult[] memory accountsStatusCheckResult,
660: StatusCheckResult[] memory vaultsStatusCheckResult
661: )
662: {
663: (bool success, bytes memory result) = address(this).delegatecall(abi.encodeCall(this.batchRevert, items));
664:
665: if (success) {
666: revert EVC_BatchPanic(); // <= FOUND
667: } else if (result.length < 4 || bytes4(result) != EVC_RevertedBatchResult.selector) {
668: revertBytes(result);
669: }
670:
671: assembly {
672: let length := mload(result)
673:
674: result := add(result, 4)
675:
676:
677: mstore(result, sub(length, 4))
678: }
679:
680: (batchItemsResult, accountsStatusCheckResult, vaultsStatusCheckResult) =
681: abi.decode(result, (BatchItemResult[], StatusCheckResult[], StatusCheckResult[]));
682: }['292']
292: function setLockdownMode(bytes19 addressPrefix, bool enabled) public payable virtual onlyOwner(addressPrefix) {
293: if (ownerLookup[addressPrefix].isLockdownMode != enabled) {
294:
295:
296:
297: if (!enabled && (executionContext.areChecksDeferred() || inPermitSelfCall())) {
298: revert EVC_NotAuthorized(); // <= FOUND
299: }
300:
301: ownerLookup[addressPrefix].isLockdownMode = enabled;
302: emit LockdownModeStatus(addressPrefix, enabled);
303: }
304: }['307']
307: function setPermitDisabledMode(
308: bytes19 addressPrefix,
309: bool enabled
310: ) public payable virtual onlyOwner(addressPrefix) {
311: if (ownerLookup[addressPrefix].isPermitDisabledMode != enabled) {
312:
313:
314:
315: if (!enabled && executionContext.areChecksDeferred()) {
316: revert EVC_NotAuthorized(); // <= FOUND
317: }
318:
319: ownerLookup[addressPrefix].isPermitDisabledMode = enabled;
320: emit PermitDisabledModeStatus(addressPrefix, enabled);
321: }
322: }['325']
325: function setNonce(
326: bytes19 addressPrefix,
327: uint256 nonceNamespace,
328: uint256 nonce
329: ) public payable virtual onlyOwner(addressPrefix) {
330: uint256 currentNonce = nonceLookup[addressPrefix][nonceNamespace];
331:
332: if (currentNonce >= nonce) {
333: revert EVC_InvalidNonce(); // <= FOUND
334: }
335:
336: nonceLookup[addressPrefix][nonceNamespace] = nonce;
337:
338: emit NonceStatus(addressPrefix, nonceNamespace, currentNonce, nonce);
339: }['344']
344: function setOperator(bytes19 addressPrefix, address operator, uint256 operatorBitField) public payable virtual {
345: address msgSender =
346: authenticateCaller({addressPrefix: addressPrefix, allowOperator: false, checkLockdownMode: false});
347:
348:
349: if (operator == address(this) || haveCommonOwnerInternal(msgSender, operator)) {
350: revert EVC_InvalidAddress(); // <= FOUND
351: }
352:
353: if (operatorLookup[addressPrefix][operator] == operatorBitField) {
354: revert EVC_InvalidOperatorStatus(); // <= FOUND
355: } else {
356: operatorLookup[addressPrefix][operator] = operatorBitField;
357:
358: emit OperatorStatus(addressPrefix, operator, operatorBitField);
359: }
360: }['365']
365: function setAccountOperator(address account, address operator, bool authorized) public payable virtual {
366: address msgSender = authenticateCaller({account: account, allowOperator: true, checkLockdownMode: false});
367:
368:
369:
370:
371:
372: address owner = haveCommonOwnerInternal(account, msgSender) ? msgSender : getAccountOwnerInternal(account);
373:
374:
375: if (owner != msgSender && operator != msgSender) {
376: revert EVC_NotAuthorized(); // <= FOUND
377: }
378:
379:
380: if (operator == address(this) || haveCommonOwnerInternal(owner, operator)) {
381: revert EVC_InvalidAddress(); // <= FOUND
382: }
383:
384: bytes19 addressPrefix = getAddressPrefixInternal(account);
385:
386:
387:
388:
389: uint256 bitMask = 1 << (uint160(owner) ^ uint160(account));
390:
391:
392:
393: uint256 oldOperatorBitField = operatorLookup[addressPrefix][operator];
394: uint256 newOperatorBitField = authorized ? oldOperatorBitField | bitMask : oldOperatorBitField & ~bitMask;
395:
396: if (oldOperatorBitField == newOperatorBitField) {
397: revert EVC_InvalidOperatorStatus(); // <= FOUND
398: } else {
399: operatorLookup[addressPrefix][operator] = newOperatorBitField;
400:
401: emit OperatorStatus(addressPrefix, operator, newOperatorBitField);
402: }
403: }['418']
418: function enableCollateral(
419: address account,
420: address vault
421: ) public payable virtual nonReentrantChecksAndControlCollateral onlyOwnerOrOperator(account) {
422: if (vault == address(this)) revert EVC_InvalidAddress(); // <= FOUND
423:
424: if (accountCollaterals[account].insert(vault)) {
425: emit CollateralStatus(account, vault, true);
426: }
427: requireAccountStatusCheck(account);
428: }['464']
464: function enableController(
465: address account,
466: address vault
467: ) public payable virtual nonReentrantChecksAndControlCollateral onlyOwnerOrOperator(account) {
468: if (vault == address(this)) revert EVC_InvalidAddress(); // <= FOUND
469:
470: if (accountControllers[account].insert(vault)) {
471: emit ControllerStatus(account, vault, true);
472: }
473: requireAccountStatusCheck(account);
474: }['487']
487: function permit(
488: address signer,
489: address sender,
490: uint256 nonceNamespace,
491: uint256 nonce,
492: uint256 deadline,
493: uint256 value,
494: bytes calldata data,
495: bytes calldata signature
496: ) public payable virtual nonReentrantChecksAndControlCollateral {
497:
498:
499: if (inPermitSelfCall() || (sender != address(0) && sender != msg.sender)) {
500: revert EVC_NotAuthorized(); // <= FOUND
501: }
502:
503: if (signer == address(0) || !isSignerValid(signer)) {
504: revert EVC_InvalidAddress(); // <= FOUND
505: }
506:
507: bytes19 addressPrefix = getAddressPrefixInternal(signer);
508:
509: if (ownerLookup[addressPrefix].isPermitDisabledMode) {
510: revert EVC_PermitDisabledMode(); // <= FOUND
511: }
512:
513: {
514: uint256 currentNonce = nonceLookup[addressPrefix][nonceNamespace];
515:
516: if (currentNonce == type(uint256).max || currentNonce != nonce) {
517: revert EVC_InvalidNonce(); // <= FOUND
518: }
519: }
520:
521: if (deadline < block.timestamp) {
522: revert EVC_InvalidTimestamp(); // <= FOUND
523: }
524:
525: if (data.length == 0) {
526: revert EVC_InvalidData(); // <= FOUND
527: }
528:
529: bytes32 permitHash = getPermitHash(signer, sender, nonceNamespace, nonce, deadline, value, data);
530:
531: if (
532: signer != recoverECDSASigner(permitHash, signature)
533: && !isValidERC1271Signature(signer, permitHash, signature)
534: ) {
535: revert EVC_NotAuthorized(); // <= FOUND
536: }
537:
538: unchecked {
539: nonceLookup[addressPrefix][nonceNamespace] = nonce + 1;
540: }
541:
542: emit NonceUsed(addressPrefix, nonceNamespace, nonce);
543:
544:
545:
546: (bool success, bytes memory result) = callWithContextInternal(address(this), signer, value, data);
547:
548: if (!success) revertBytes(result);
549: }['572']
572: function controlCollateral(
573: address targetCollateral,
574: address onBehalfOfAccount,
575: uint256 value,
576: bytes calldata data
577: )
578: public
579: payable
580: virtual
581: nonReentrantChecksAndControlCollateral
582: onlyController(onBehalfOfAccount)
583: returns (bytes memory result)
584: {
585: if (!accountCollaterals[onBehalfOfAccount].contains(targetCollateral)) {
586: revert EVC_NotAuthorized(); // <= FOUND
587: }
588:
589: EC contextCache = executionContext;
590: executionContext = contextCache.setChecksDeferred().setControlCollateralInProgress();
591:
592: bool success;
593: (success, result) = callWithContextInternal(targetCollateral, onBehalfOfAccount, value, data);
594:
595: if (!success) revertBytes(result);
596:
597: restoreExecutionContext(contextCache);
598: }['620']
620: function batchRevert(BatchItem[] calldata items) public payable virtual nonReentrantChecksAndControlCollateral {
621: BatchItemResult[] memory batchItemsResult;
622: StatusCheckResult[] memory accountsStatusCheckResult;
623: StatusCheckResult[] memory vaultsStatusCheckResult;
624:
625: EC contextCache = executionContext;
626:
627: if (contextCache.areChecksDeferred()) {
628: revert EVC_SimulationBatchNested(); // <= FOUND
629: }
630:
631: executionContext = contextCache.setChecksDeferred().setSimulationInProgress();
632:
633: uint256 length = items.length;
634: batchItemsResult = new BatchItemResult[](length);
635:
636: for (uint256 i; i < length; ++i) {
637: BatchItem calldata item = items[i];
638: (batchItemsResult[i].success, batchItemsResult[i].result) =
639: callWithAuthenticationInternal(item.targetContract, item.onBehalfOfAccount, item.value, item.data);
640: }
641:
642: executionContext = contextCache.setChecksInProgress().setOnBehalfOfAccount(address(0));
643:
644: accountsStatusCheckResult = checkStatusAllWithResult(SetType.Account);
645: vaultsStatusCheckResult = checkStatusAllWithResult(SetType.Vault);
646:
647: executionContext = contextCache;
648:
649: revert EVC_RevertedBatchResult(batchItemsResult, accountsStatusCheckResult, vaultsStatusCheckResult); // <= FOUND
650: }[NonCritical-8] Private and internal state variables should have a preceding _ in their name unless they are constants
Add a preceding underscore to the state variable name, take care to refactor where there variables are read/wrote
Num of instances: 6
Click to show findings
['43']
43: uint256 internal immutable CACHED_CHAIN_ID; // <= FOUND['44']
44: bytes32 internal immutable CACHED_DOMAIN_SEPARATOR; // <= FOUND['14']
14: IEVC internal immutable evc; // <= FOUND['18']
18: enum SetType {
19: Account,
20: Vault
21: }
22:
23: EC internal executionContext; // <= FOUND['24']
24: SetStorage internal accountStatusChecks; // <= FOUND['25']
25: SetStorage internal vaultStatusChecks; // <= FOUNDConsider spreading these lines over multiple lines to aid in readability and the support of VIM users everywhere.
Num of instances: 4
Click to show findings
['160']
160: /// definition) permit message. To support permit functionality by default, note that the logic was inverted here. To // <= FOUND['40']
40: "Permit(address signer,address sender,uint256 nonceNamespace,uint256 nonce,uint256 deadline,uint256 value,bytes data)" // <= FOUND['982']
982: success && result.length == 32 && abi.decode(result, (bytes32)) == bytes32(IVault.checkVaultStatus.selector); // <= FOUND['1127']
1127: /// https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/SignatureChecker.sol // <= FOUNDTry to index as much as three variables in event declarations as this is more gas efficient when done on value type variables (uint, address etc) however not for bytes and string variables
Num of instances: 3
Click to show findings
['30']
30: event NonceStatus( // <= FOUND
31: bytes19 indexed addressPrefix, uint256 indexed nonceNamespace, uint256 oldNonce, uint256 newNonce
32: );['50']
50: event CollateralStatus(address indexed account, address indexed collateral, bool enabled); // <= FOUND['56']
56: event ControllerStatus(address indexed account, address indexed controller, bool enabled); // <= FOUNDContracts should expose all public and external functions through interfaces. This practice ensures a clear and consistent definition of how the contract can be interacted with, promoting better transparency and integration.
Num of instances: 23
Click to show findings
['28']
28: function EVC() external view returns (address) ['203']
203: function getRawExecutionContext() external view returns (uint256 context) ['64']
64: function getCurrentOnBehalfOfAccount(address controllerToCheck)
65: external
66: view
67: returns (address onBehalfOfAccount, bool controllerEnabled)
68: ['225']
225: function areChecksDeferred() external view returns (bool) ['230']
230: function areChecksInProgress() external view returns (bool) ['235']
235: function isControlCollateralInProgress() external view returns (bool) ['240']
240: function isOperatorAuthenticated() external view returns (bool) ['245']
245: function isSimulationInProgress() external view returns (bool) ['252']
252: function haveCommonOwner(address account, address otherAccount) external pure returns (bool) ['257']
257: function getAddressPrefix(address account) external pure returns (bytes19) ['262']
262: function getAccountOwner(address account) external view returns (address) ['267']
267: function isLockdownMode(bytes19 addressPrefix) external view returns (bool) ['272']
272: function isPermitDisabledMode(bytes19 addressPrefix) external view returns (bool) ['277']
277: function getNonce(bytes19 addressPrefix, uint256 nonceNamespace) external view returns (uint256) ['282']
282: function getOperator(bytes19 addressPrefix, address operator) external view returns (uint256) ['287']
287: function isAccountOperatorAuthorized(address account, address operator) external view returns (bool) ['408']
408: function getCollaterals(address account) external view returns (address[] memory) ['413']
413: function isCollateralEnabled(address account, address vault) external view returns (bool) ['454']
454: function getControllers(address account) external view returns (address[] memory) ['459']
459: function isControllerEnabled(address account, address vault) external view returns (bool) ['687']
687: function getLastAccountStatusCheckTimestamp(address account) external view nonReentrantChecks returns (uint256) ['692']
692: function isAccountStatusCheckDeferred(address account) external view nonReentrantChecks returns (bool) ['719']
719: function isVaultStatusCheckDeferred(address vault) external view nonReentrantChecks returns (bool) The following order should be used within contracts
constructor
receive function (if exists)
fallback function (if exists)
external
public
internal
private
Rearrange the contract functions and contructors to fit this ordering
Num of instances: 1
Click to show findings
['18']
18: contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC // <= FOUNDDouble type casting should be avoided in Solidity contracts to prevent unintended consequences and ensure accurate data representation. Performing multiple type casts in succession can lead to unexpected truncation, rounding errors, or loss of precision, potentially compromising the contract's functionality and reliability. Furthermore, double type casting can make the code less readable and harder to maintain, increasing the likelihood of errors and misunderstandings during development and debugging. To ensure precise and consistent data handling, developers should use appropriate data types and avoid unnecessary or excessive type casting, promoting a more robust and dependable contract execution.
Num of instances: 2
Click to show findings
['812']
812: address phantomAccount = address(uint160(uint152(addressPrefix)) << ACCOUNT_ID_OFFSET); // <= FOUND['1187']
1187: return bytes19(uint152(uint160(account) >> ACCOUNT_ID_OFFSET)); // <= FOUNDIn Solidity, when msg.sender plays a crucial role in a function's logic, it's important for transparency and auditability that any events emitted by this function include msg.sender as a parameter. This practice enhances the traceability and accountability of transactions, allowing users and external observers to easily track who initiated a particular action. Including msg.sender in event logs helps in creating a clear and verifiable record of interactions with the contract, thereby increasing user trust and facilitating easier debugging and analysis of contract behavior. It's a key aspect of writing clear, transparent, and user-friendly smart contracts.
Num of instances: 2
Click to show findings
['487']
487: function permit(
488: address signer,
489: address sender,
490: uint256 nonceNamespace,
491: uint256 nonce,
492: uint256 deadline,
493: uint256 value,
494: bytes calldata data,
495: bytes calldata signature
496: ) public payable virtual nonReentrantChecksAndControlCollateral {
497:
498:
499: if (inPermitSelfCall() || (sender != address(0) && sender != msg.sender)) { // <= FOUND
500: revert EVC_NotAuthorized();
501: }
502:
503: if (signer == address(0) || !isSignerValid(signer)) {
504: revert EVC_InvalidAddress();
505: }
506:
507: bytes19 addressPrefix = getAddressPrefixInternal(signer);
508:
509: if (ownerLookup[addressPrefix].isPermitDisabledMode) {
510: revert EVC_PermitDisabledMode();
511: }
512:
513: {
514: uint256 currentNonce = nonceLookup[addressPrefix][nonceNamespace];
515:
516: if (currentNonce == type(uint256).max || currentNonce != nonce) {
517: revert EVC_InvalidNonce();
518: }
519: }
520:
521: if (deadline < block.timestamp) {
522: revert EVC_InvalidTimestamp();
523: }
524:
525: if (data.length == 0) {
526: revert EVC_InvalidData();
527: }
528:
529: bytes32 permitHash = getPermitHash(signer, sender, nonceNamespace, nonce, deadline, value, data);
530:
531: if (
532: signer != recoverECDSASigner(permitHash, signature)
533: && !isValidERC1271Signature(signer, permitHash, signature)
534: ) {
535: revert EVC_NotAuthorized();
536: }
537:
538: unchecked {
539: nonceLookup[addressPrefix][nonceNamespace] = nonce + 1;
540: }
541:
542: emit NonceUsed(addressPrefix, nonceNamespace, nonce); // <= FOUND
543:
544:
545:
546: (bool success, bytes memory result) = callWithContextInternal(address(this), signer, value, data);
547:
548: if (!success) revertBytes(result);
549: }['827']
827: function callWithContextInternal(
828: address targetContract,
829: address onBehalfOfAccount,
830: uint256 value,
831: bytes calldata data
832: ) internal virtual returns (bool success, bytes memory result) {
833: if (value == type(uint256).max) {
834: value = address(this).balance;
835: } else if (value > address(this).balance) {
836: revert EVC_InvalidValue();
837: }
838:
839: EC contextCache = executionContext;
840: address msgSender = _msgSender();
841:
842:
843:
844:
845:
846:
847:
848:
849: if (
850: haveCommonOwnerInternal(onBehalfOfAccount, msgSender) || targetContract == msg.sender // <= FOUND
851: || targetContract == address(this) || contextCache.isControlCollateralInProgress()
852: ) {
853: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).clearOperatorAuthenticated();
854: } else {
855: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).setOperatorAuthenticated();
856: }
857:
858: emit CallWithContext( // <= FOUND
859: msgSender, getAddressPrefixInternal(onBehalfOfAccount), onBehalfOfAccount, targetContract, bytes4(data)
860: );
861:
862: (success, result) = targetContract.call{value: value}(data);
863:
864: executionContext = contextCache;
865: }[NonCritical-15] A function which defines named returns in it's declaration doesn't need to use return
Refacter the code to assign to the named return variables rather than using a return statement
Num of instances: 3
Click to show findings
['1091']
1091: function recoverECDSASigner(bytes32 hash, bytes memory signature) internal pure returns (address signer) {
1092: if (signature.length != 65) return address(0); // <= FOUND
1093:
1094: bytes32 r;
1095: bytes32 s;
1096: uint8 v;
1097:
1098:
1099:
1100:
1101: assembly {
1102: r := mload(add(signature, 0x20))
1103: s := mload(add(signature, 0x40))
1104: v := byte(0, mload(add(signature, 0x60)))
1105: }
1106:
1107:
1108:
1109:
1110:
1111:
1112:
1113:
1114:
1115:
1116: if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) {
1117: return address(0); // <= FOUND
1118: }
1119:
1120:
1121: signer = ecrecover(hash, v, r, s);
1122: }['1134']
1134: function isValidERC1271Signature(
1135: address signer,
1136: bytes32 hash,
1137: bytes memory signature
1138: ) internal view returns (bool isValid) {
1139: if (signer.code.length == 0) return false; // <= FOUND
1140:
1141: (bool success, bytes memory result) =
1142: signer.staticcall(abi.encodeCall(IERC1271.isValidSignature, (hash, signature)));
1143:
1144: isValid = success && result.length == 32
1145: && abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector);
1146: }['1206']
1206: function isAccountOperatorAuthorizedInternal(
1207: address account,
1208: address operator
1209: ) internal view returns (bool isAuthorized) {
1210: address owner = getAccountOwnerInternal(account);
1211:
1212:
1213: if (owner == address(0)) return false; // <= FOUND
1214:
1215: bytes19 addressPrefix = getAddressPrefixInternal(account);
1216:
1217:
1218:
1219:
1220: uint256 bitMask = 1 << (uint160(owner) ^ uint160(account));
1221:
1222: return operatorLookup[addressPrefix][operator] & bitMask != 0; // <= FOUND
1223: }If these serve no purpose, they should be safely removed
Num of instances: 2
Click to show findings
['36']
36: modifier callThroughEVC() virtual { // <= FOUND
37: if (msg.sender == address(evc)) {
38: _;
39: } else {
40: address _evc = address(evc);
41:
42: assembly {
43: mstore(0, 0x1f8b521500000000000000000000000000000000000000000000000000000000)
44: mstore(4, address())
45: mstore(36, caller())
46: mstore(68, callvalue())
47: mstore(100, 128)
48: mstore(132, calldatasize())
49: calldatacopy(164, 0, calldatasize())
50:
51:
52:
53: mstore(add(164, calldatasize()), 0)
54: let result := call(gas(), _evc, callvalue(), 0, add(164, and(add(calldatasize(), 31), not(31))), 0, 0)
55:
56: returndatacopy(0, 0, returndatasize())
57: switch result
58: case 0 { revert(0, returndatasize()) }
59: default { return(64, sub(returndatasize(), 64)) }
60: }
61: }
62: }['66']
66: modifier onlyEVCWithChecksInProgress() virtual { // <= FOUND
67: if (msg.sender != address(evc) || !evc.areChecksInProgress()) {
68: revert NotAuthorized();
69: }
70:
71: _;
72: }Putting constants on the left side of a comparison operator like == or < is a best practice known as "Yoda conditions", which can help prevent accidental assignment instead of comparison. In some programming languages, if a variable is mistakenly put on the left with a single = instead of ==, it assigns the constant's value to the variable without any compiler error. However, doing this with the constant on the left would generate an error, as constants cannot be assigned values. Although Solidity's static typing system prevents accidental assignments within conditionals, adopting this practice enhances code readability and consistency, especially when developers are working across multiple languages that support this convention.
Num of instances: 9
Click to show findings
['525']
525: if (data.length == 0) // <= FOUND['1229']
1229: if (errMsg.length != 0) // <= FOUND['76']
76: if (numElements == 0) // <= FOUND['144']
144: if (searchIndex == 0) // <= FOUND['184']
184: if (index1 == 0) // <= FOUND['888']
888: if (value != 0) // <= FOUND['127']
127: if (numElements == 1) // <= FOUND['140']
140: if (numOfControllers != 1) // <= FOUND['1116']
1116: if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) // <= FOUNDSuch instances can be replaced with unnamed returns
Num of instances: 1
Click to show findings
['1206']
1206: function isAccountOperatorAuthorizedInternal( // <= FOUND
1207: address account,
1208: address operator
1209: ) internal view returns (bool isAuthorized) { // <= FOUND
1210: address owner = getAccountOwnerInternal(account);
1211:
1212:
1213: if (owner == address(0)) return false;
1214:
1215: bytes19 addressPrefix = getAddressPrefixInternal(account);
1216:
1217:
1218:
1219:
1220: uint256 bitMask = 1 << (uint160(owner) ^ uint160(account));
1221:
1222: return operatorLookup[addressPrefix][operator] & bitMask != 0;
1223: }Emitting an event within initializer functions in Solidity is a best practice for providing transparency and traceability. Initializer functions set the initial state and values of an upgradeable contract. Emitting an event during initialization allows anyone to verify and audit the initial state of the contract via the transaction logs. This can be particularly useful for verifying the parameters set during initialization, tracking the contract's deployment, and troubleshooting or debugging. Therefore, developers should include an event emission in their initializer functions, providing a clear record of the contract's initialization and enhancing the contract's transparency and security.
Num of instances: 1
Click to show findings
['57']
57: function initialize(SetStorage storage setStorage) internal { // <= FOUND
58: setStorage.stamp = DUMMY_STAMP;
59:
60: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < SET_MAX_ELEMENTS; ++i) {
61: setStorage.elements[i].stamp = DUMMY_STAMP;
62: }
63: }Magic numbers should be avoided in Solidity code to enhance readability, maintainability, and reduce the likelihood of errors. Magic numbers are hard-coded values with no clear meaning or context, which can create confusion and make the code harder to understand for developers. Using well-defined constants or variables with descriptive names instead of magic numbers not only clarifies the purpose and significance of the value but also simplifies code updates and modifications.
Num of instances: 6
Click to show findings
['943']
943: isValid = success && result.length == 32 // <= FOUND
944: && abi.decode(result, (bytes32)) == bytes32(IVault.checkAccountStatus.selector);['981']
981: isValid =
982: success && result.length == 32 && abi.decode(result, (bytes32)) == bytes32(IVault.checkVaultStatus.selector); // <= FOUND['1144']
1144: isValid = success && result.length == 32 // <= FOUND
1145: && abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector);['667']
667: } else if (result.length < 4 || bytes4(result) != EVC_RevertedBatchResult.selector) { // <= FOUND['59']
59: default { return(64, sub(returndatasize(), 64)) } // <= FOUND['1092']
1092: if (signature.length != 65) return address(0); // <= FOUNDIt's crucial to leverage the right features for the appropriate contexts in Solidity, despite the compiler's ability to correct common developer mistakes. Both constant and immutable variables have distinct uses. Constant variables are best suited for hard-coded, literal values within your contract, where the value doesn't need to be computed or modified.
On the other hand, immutable variables are ideal for situations where the value might be the result of an expression or received from a constructor. While both serve to define unchanging variables, they function differently and their proper utilization contributes to clearer, more efficient code. Remember, even if the compiler can correct certain mistakes, best practices dictate using the correct feature for the task at hand.
Num of instances: 3
Click to show findings
['33']
33: bytes32 internal constant HASHED_NAME = keccak256(bytes(name)); // <= FOUND['34']
34: bytes32 internal constant HASHED_VERSION = keccak256(bytes(version)); // <= FOUND['39']
39: bytes32 internal constant PERMIT_TYPEHASH = keccak256( // <= FOUNDWhen developing smart contracts in Solidity, it's crucial to validate the inputs of your functions. This includes ensuring that the bytes parameters are not empty, especially when they represent crucial data such as addresses, identifiers, or raw data that the contract needs to process.
Missing empty bytes checks can lead to unexpected behaviour in your contract. For instance, certain operations might fail, produce incorrect results, or consume unnecessary gas when performed with empty bytes. Moreover, missing input validation can potentially expose your contract to malicious activity, including exploitation of unhandled edge cases.
To mitigate these issues, always validate that bytes parameters are not empty when the logic of your contract requires it.
Num of instances: 14
Click to show findings
['267']
267: function isLockdownMode(bytes19 addressPrefix) external view returns (bool) {
268: return ownerLookup[addressPrefix].isLockdownMode;
269: }['272']
272: function isPermitDisabledMode(bytes19 addressPrefix) external view returns (bool) {
273: return ownerLookup[addressPrefix].isPermitDisabledMode;
274: }['277']
277: function getNonce(bytes19 addressPrefix, uint256 nonceNamespace) external view returns (uint256) {
278: return nonceLookup[addressPrefix][nonceNamespace];
279: }['282']
282: function getOperator(bytes19 addressPrefix, address operator) external view returns (uint256) {
283: return operatorLookup[addressPrefix][operator];
284: }['292']
292: function setLockdownMode(bytes19 addressPrefix, bool enabled) public payable virtual onlyOwner(addressPrefix) {
293: if (ownerLookup[addressPrefix].isLockdownMode != enabled) {
294:
295:
296:
297: if (!enabled && (executionContext.areChecksDeferred() || inPermitSelfCall())) {
298: revert EVC_NotAuthorized();
299: }
300:
301: ownerLookup[addressPrefix].isLockdownMode = enabled;
302: emit LockdownModeStatus(addressPrefix, enabled);
303: }
304: }['307']
307: function setPermitDisabledMode(
308: bytes19 addressPrefix,
309: bool enabled
310: ) public payable virtual onlyOwner(addressPrefix) {
311: if (ownerLookup[addressPrefix].isPermitDisabledMode != enabled) {
312:
313:
314:
315: if (!enabled && executionContext.areChecksDeferred()) {
316: revert EVC_NotAuthorized();
317: }
318:
319: ownerLookup[addressPrefix].isPermitDisabledMode = enabled;
320: emit PermitDisabledModeStatus(addressPrefix, enabled);
321: }
322: }['325']
325: function setNonce(
326: bytes19 addressPrefix,
327: uint256 nonceNamespace,
328: uint256 nonce
329: ) public payable virtual onlyOwner(addressPrefix) {
330: uint256 currentNonce = nonceLookup[addressPrefix][nonceNamespace];
331:
332: if (currentNonce >= nonce) {
333: revert EVC_InvalidNonce();
334: }
335:
336: nonceLookup[addressPrefix][nonceNamespace] = nonce;
337:
338: emit NonceStatus(addressPrefix, nonceNamespace, currentNonce, nonce);
339: }['344']
344: function setOperator(bytes19 addressPrefix, address operator, uint256 operatorBitField) public payable virtual {
345: address msgSender =
346: authenticateCaller({addressPrefix: addressPrefix, allowOperator: false, checkLockdownMode: false});
347:
348:
349: if (operator == address(this) || haveCommonOwnerInternal(msgSender, operator)) {
350: revert EVC_InvalidAddress();
351: }
352:
353: if (operatorLookup[addressPrefix][operator] == operatorBitField) {
354: revert EVC_InvalidOperatorStatus();
355: } else {
356: operatorLookup[addressPrefix][operator] = operatorBitField;
357:
358: emit OperatorStatus(addressPrefix, operator, operatorBitField);
359: }
360: }['554']
554: function call(
555: address targetContract,
556: address onBehalfOfAccount,
557: uint256 value,
558: bytes calldata data
559: ) public payable virtual nonReentrantChecksAndControlCollateral returns (bytes memory result) {
560: EC contextCache = executionContext;
561: executionContext = contextCache.setChecksDeferred();
562:
563: bool success;
564: (success, result) = callWithAuthenticationInternal(targetContract, onBehalfOfAccount, value, data);
565:
566: if (!success) revertBytes(result);
567:
568: restoreExecutionContext(contextCache);
569: }['572']
572: function controlCollateral(
573: address targetCollateral,
574: address onBehalfOfAccount,
575: uint256 value,
576: bytes calldata data
577: )
578: public
579: payable
580: virtual
581: nonReentrantChecksAndControlCollateral
582: onlyController(onBehalfOfAccount)
583: returns (bytes memory result)
584: {
585: if (!accountCollaterals[onBehalfOfAccount].contains(targetCollateral)) {
586: revert EVC_NotAuthorized();
587: }
588:
589: EC contextCache = executionContext;
590: executionContext = contextCache.setChecksDeferred().setControlCollateralInProgress();
591:
592: bool success;
593: (success, result) = callWithContextInternal(targetCollateral, onBehalfOfAccount, value, data);
594:
595: if (!success) revertBytes(result);
596:
597: restoreExecutionContext(contextCache);
598: }['807']
807: function authenticateCaller(
808: bytes19 addressPrefix,
809: bool allowOperator,
810: bool checkLockdownMode
811: ) internal virtual returns (address) {
812: address phantomAccount = address(uint160(uint152(addressPrefix)) << ACCOUNT_ID_OFFSET);
813:
814: return authenticateCaller({
815: account: phantomAccount,
816: allowOperator: allowOperator,
817: checkLockdownMode: checkLockdownMode
818: });
819: }['827']
827: function callWithContextInternal(
828: address targetContract,
829: address onBehalfOfAccount,
830: uint256 value,
831: bytes calldata data
832: ) internal virtual returns (bool success, bytes memory result) {
833: if (value == type(uint256).max) {
834: value = address(this).balance;
835: } else if (value > address(this).balance) {
836: revert EVC_InvalidValue();
837: }
838:
839: EC contextCache = executionContext;
840: address msgSender = _msgSender();
841:
842:
843:
844:
845:
846:
847:
848:
849: if (
850: haveCommonOwnerInternal(onBehalfOfAccount, msgSender) || targetContract == msg.sender
851: || targetContract == address(this) || contextCache.isControlCollateralInProgress()
852: ) {
853: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).clearOperatorAuthenticated();
854: } else {
855: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).setOperatorAuthenticated();
856: }
857:
858: emit CallWithContext(
859: msgSender, getAddressPrefixInternal(onBehalfOfAccount), onBehalfOfAccount, targetContract, bytes4(data)
860: );
861:
862: (success, result) = targetContract.call{value: value}(data);
863:
864: executionContext = contextCache;
865: }['877']
877: function callWithAuthenticationInternal(
878: address targetContract,
879: address onBehalfOfAccount,
880: uint256 value,
881: bytes calldata data
882: ) internal virtual returns (bool success, bytes memory result) {
883: if (targetContract == address(this)) {
884: if (onBehalfOfAccount != address(0)) {
885: revert EVC_InvalidAddress();
886: }
887:
888: if (value != 0) {
889: revert EVC_InvalidValue();
890: }
891:
892:
893: (success, result) = address(this).delegatecall(data);
894: } else {
895:
896:
897: if (targetContract != msg.sender) {
898: authenticateCaller({account: onBehalfOfAccount, allowOperator: true, checkLockdownMode: true});
899: }
900:
901: (success, result) = callWithContextInternal(targetContract, onBehalfOfAccount, value, data);
902: }
903: }['1056']
1056: function getPermitHash(
1057: address signer,
1058: address sender,
1059: uint256 nonceNamespace,
1060: uint256 nonce,
1061: uint256 deadline,
1062: uint256 value,
1063: bytes calldata data
1064: ) internal view returns (bytes32 permitHash) {
1065: bytes32 domainSeparator =
1066: block.chainid == CACHED_CHAIN_ID ? CACHED_DOMAIN_SEPARATOR : calculateDomainSeparator();
1067:
1068: bytes32 structHash = keccak256(
1069: abi.encode(PERMIT_TYPEHASH, signer, sender, nonceNamespace, nonce, deadline, value, keccak256(data))
1070: );
1071:
1072:
1073:
1074: assembly ("memory-safe") {
1075: mstore(0x00, "\x19\x01")
1076: mstore(0x02, domainSeparator)
1077: mstore(0x22, structHash)
1078: permitHash := keccak256(0x00, 0x42)
1079: mstore(0x22, 0)
1080: }
1081: }In Solidity, when designing functions that return boolean values, it's crucial for clarity and maintainability to explicitly handle both true and false return scenarios. If a function is intended to return true under certain conditions, it should also explicitly return false when these conditions are not met, and vice versa. This approach eliminates ambiguity and makes the code's intent more transparent. Explicitly handling all possible outcomes of a boolean function ensures that future modifications or extensions of the contract do not unintentionally alter its logic. It contributes to better readability, easier debugging, and reduces the risk of bugs related to unintended fall-through cases.
Num of instances: 1
Click to show findings
['1134']
1134: function isValidERC1271Signature(
1135: address signer,
1136: bytes32 hash,
1137: bytes memory signature
1138: ) internal view returns (bool isValid) {
1139: if (signer.code.length == 0) return false; // <= FOUND
1140:
1141: (bool success, bytes memory result) =
1142: signer.staticcall(abi.encodeCall(IERC1271.isValidSignature, (hash, signature)));
1143:
1144: isValid = success && result.length == 32
1145: && abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector);
1146: }[NonCritical-24] Use OpenZeppelin's ReentrancyGuard/ReentrancyGuardUpgradeable rather than writing your own
Leveraging OpenZeppelin's ReentrancyGuard or ReentrancyGuardUpgradeable is generally recommended over creating your own versions. The OpenZeppelin modules have undergone rigorous optimization for gas efficiency and have been comprehensively tested for potential security vulnerabilities. As a result, they provide a high degree of reliability and performance. Writing your own reentrancy guard mechanism might introduce unseen bugs or security loopholes, and likely be less gas efficient. Therefore, it's beneficial to rely on the tried-and-tested solutions offered by OpenZeppelin's established and widely-accepted smart contract library.
Num of instances: 3
Click to show findings
['153']
153: modifier nonReentrantChecks() { // <= FOUND
154: if (executionContext.areChecksInProgress()) {
155: revert EVC_ChecksReentrancy();
156: }
157:
158: _;
159: }['163']
163: modifier nonReentrantChecksAndControlCollateral() { // <= FOUND
164: {
165: EC context = executionContext;
166:
167: if (context.areChecksInProgress()) {
168: revert EVC_ChecksReentrancy();
169: }
170:
171: if (context.isControlCollateralInProgress()) {
172: revert EVC_ControlCollateralReentrancy();
173: }
174: }
175:
176: _;
177: }['182']
182: modifier nonReentrantChecksAcquireLock() { // <= FOUND
183: EC contextCache = executionContext;
184:
185: if (contextCache.areChecksInProgress()) {
186: revert EVC_ChecksReentrancy();
187: }
188:
189: executionContext = contextCache.setChecksInProgress().setOnBehalfOfAccount(address(0));
190:
191: _;
192:
193: executionContext = contextCache;
194: }Manipulating data directly at the free memory pointer location without subsequently adjusting the pointer can lead to unwanted data remnants, or "dirty bits", in that memory spot. This can cause challenges for the Solidity optimizer, making it difficult to determine if memory cleaning is required before reuse, potentially resulting in less efficient optimization. To mitigate this issue, it's advised to always update the free memory pointer following any data write operation. Furthermore, using the assembly ("memory-safe") { ... } annotation will clearly indicate to the optimizer the sections of your code that are memory-safe, improving code efficiency and reducing the potential for errors.
Num of instances: 1
Click to show findings
['1101']
1101: assembly {
1102: r := mload(add(signature, 0x20))
1103: s := mload(add(signature, 0x40)) // <= FOUND
1104: v := byte(0, mload(add(signature, 0x60)))
1105: }Using the .call method in Solidity enables direct communication with an address, bypassing function existence checks, type checking, and argument packing. While this can save gas and provide flexibility, it can also introduce security risks and potential errors. The absence of these checks can lead to unexpected behavior if the callee contract's interface changes or if the input parameters are not crafted with care. The resolution to these issues is to use Solidity's high-level interface for calling functions when possible, as it automatically manages these aspects. If using .call is necessary, ensure that the inputs are carefully validated and that awareness of the called contract's behavior is maintained.
Num of instances: 4
Click to show findings
['940']
940: (success, result) =
941: controller.call(abi.encodeCall(IVault.checkAccountStatus, (account, accountCollaterals[account].get()))); // <= FOUND['979']
979: (success, result) = vault.call(abi.encodeCall(IVault.checkVaultStatus, ())); // <= FOUND['862']
862: (success, result) = targetContract.call{value: value}(data); // <= FOUND['43']
43: mstore(0, 0x1f8b521500000000000000000000000000000000000000000000000000000000)
44: mstore(4, address())
45: mstore(36, caller())
46: mstore(68, callvalue())
47: mstore(100, 128)
48: mstore(132, calldatasize())
49: calldatacopy(164, 0, calldatasize())
50:
51:
52:
53: mstore(add(164, calldatasize()), 0)
54: let result := call(gas(), _evc, callvalue(), 0, add(164, and(add(calldatasize(), 31), not(31))), 0, 0) // <= FOUND
55:
56: returndatacopy(0, 0, returndatasize())
57: switch result
58: case 0 { revert(0, returndatasize()) }Cyclomatic complexity is a software metric used to measure the complexity of a program. It quantifies the number of linearly independent paths through a program's source code, giving an idea of how complex the control flow is. High cyclomatic complexity may indicate a higher risk of defects and can make the code harder to understand, test, and maintain. It often suggests that a function or method is trying to do too much, and a refactor might be needed. By breaking down complex functions into smaller, more focused pieces, you can improve readability, ease of testing, and overall maintainability.
Num of instances: 5
Click to show findings
['365']
365: function setAccountOperator(address account, address operator, bool authorized) public payable virtual { // <= FOUND
366: address msgSender = authenticateCaller({account: account, allowOperator: true, checkLockdownMode: false});
367:
368:
369:
370:
371:
372: address owner = haveCommonOwnerInternal(account, msgSender) ? msgSender : getAccountOwnerInternal(account);
373:
374:
375: if (owner != msgSender && operator != msgSender) {
376: revert EVC_NotAuthorized();
377: }
378:
379:
380: if (operator == address(this) || haveCommonOwnerInternal(owner, operator)) {
381: revert EVC_InvalidAddress();
382: }
383:
384: bytes19 addressPrefix = getAddressPrefixInternal(account);
385:
386:
387:
388:
389: uint256 bitMask = 1 << (uint160(owner) ^ uint160(account));
390:
391:
392:
393: uint256 oldOperatorBitField = operatorLookup[addressPrefix][operator];
394: uint256 newOperatorBitField = authorized ? oldOperatorBitField | bitMask : oldOperatorBitField & ~bitMask;
395:
396: if (oldOperatorBitField == newOperatorBitField) {
397: revert EVC_InvalidOperatorStatus();
398: } else {
399: operatorLookup[addressPrefix][operator] = newOperatorBitField;
400:
401: emit OperatorStatus(addressPrefix, operator, newOperatorBitField);
402: }
403: }[]
487: function permit(
488: address signer,
489: address sender,
490: uint256 nonceNamespace,
491: uint256 nonce,
492: uint256 deadline,
493: uint256 value,
494: bytes calldata data,
495: bytes calldata signature
496: ) public payable virtual nonReentrantChecksAndControlCollateral {
497:
498:
499: if (inPermitSelfCall() || (sender != address(0) && sender != msg.sender)) {
500: revert EVC_NotAuthorized();
501: }
502:
503: if (signer == address(0) || !isSignerValid(signer)) {
504: revert EVC_InvalidAddress();
505: }
506:
507: bytes19 addressPrefix = getAddressPrefixInternal(signer);
508:
509: if (ownerLookup[addressPrefix].isPermitDisabledMode) {
510: revert EVC_PermitDisabledMode();
511: }
512:
513: {
514: uint256 currentNonce = nonceLookup[addressPrefix][nonceNamespace];
515:
516: if (currentNonce == type(uint256).max || currentNonce != nonce) {
517: revert EVC_InvalidNonce();
518: }
519: }
520:
521: if (deadline < block.timestamp) {
522: revert EVC_InvalidTimestamp();
523: }
524:
525: if (data.length == 0) {
526: revert EVC_InvalidData();
527: }
528:
529: bytes32 permitHash = getPermitHash(signer, sender, nonceNamespace, nonce, deadline, value, data);
530:
531: if (
532: signer != recoverECDSASigner(permitHash, signature)
533: && !isValidERC1271Signature(signer, permitHash, signature)
534: ) {
535: revert EVC_NotAuthorized();
536: }
537:
538: unchecked {
539: nonceLookup[addressPrefix][nonceNamespace] = nonce + 1;
540: }
541:
542: emit NonceUsed(addressPrefix, nonceNamespace, nonce);
543:
544:
545:
546: (bool success, bytes memory result) = callWithContextInternal(address(this), signer, value, data);
547:
548: if (!success) revertBytes(result);
549: }[]
759: function authenticateCaller(
760: address account,
761: bool allowOperator,
762: bool checkLockdownMode
763: ) internal virtual returns (address) {
764: bytes19 addressPrefix = getAddressPrefixInternal(account);
765: address owner = ownerLookup[addressPrefix].owner;
766: bool lockdownMode = ownerLookup[addressPrefix].isLockdownMode;
767: address msgSender = _msgSender();
768: bool authenticated = false;
769:
770:
771: if (haveCommonOwnerInternal(account, msgSender)) {
772:
773: if (owner == address(0)) {
774: ownerLookup[addressPrefix].owner = msgSender;
775: emit OwnerRegistered(addressPrefix, msgSender);
776: authenticated = true;
777: } else if (owner == msgSender) {
778: authenticated = true;
779: }
780: }
781:
782:
783: if (!authenticated && allowOperator && isAccountOperatorAuthorizedInternal(account, msgSender)) {
784: authenticated = true;
785: }
786:
787:
788: if (!authenticated) {
789: revert EVC_NotAuthorized();
790: }
791:
792:
793: if (checkLockdownMode && lockdownMode) {
794: revert EVC_LockdownMode();
795: }
796:
797: return msgSender;
798: }[]
877: function callWithAuthenticationInternal(
878: address targetContract,
879: address onBehalfOfAccount,
880: uint256 value,
881: bytes calldata data
882: ) internal virtual returns (bool success, bytes memory result) {
883: if (targetContract == address(this)) {
884: if (onBehalfOfAccount != address(0)) {
885: revert EVC_InvalidAddress();
886: }
887:
888: if (value != 0) {
889: revert EVC_InvalidValue();
890: }
891:
892:
893: (success, result) = address(this).delegatecall(data);
894: } else {
895:
896:
897: if (targetContract != msg.sender) {
898: authenticateCaller({account: onBehalfOfAccount, allowOperator: true, checkLockdownMode: true});
899: }
900:
901: (success, result) = callWithContextInternal(targetContract, onBehalfOfAccount, value, data);
902: }
903: }['110']
110: function remove(SetStorage storage setStorage, address element) internal returns (bool) { // <= FOUND
111: address firstElement = setStorage.firstElement;
112: uint256 numElements = setStorage.numElements;
113: uint80 metadata = setStorage.metadata;
114:
115: if (numElements == 0) return false;
116:
117: uint256 searchIndex;
118: if (firstElement != element) {
119: for (searchIndex = EMPTY_ELEMENT_OFFSET; searchIndex < numElements; ++searchIndex) {
120: if (setStorage.elements[searchIndex].value == element) break;
121: }
122:
123: if (searchIndex == numElements) return false;
124: }
125:
126:
127: if (numElements == 1) {
128: setStorage.numElements = 0;
129: setStorage.firstElement = address(0);
130: setStorage.metadata = metadata;
131: setStorage.stamp = DUMMY_STAMP;
132: return true;
133: }
134:
135: uint256 lastIndex;
136: unchecked {
137: lastIndex = numElements - 1;
138: }
139:
140:
141:
142: ElementStorage storage lastElement = setStorage.elements[lastIndex];
143: if (searchIndex != lastIndex) {
144: if (searchIndex == 0) {
145: setStorage.firstElement = lastElement.value;
146: setStorage.numElements = uint8(lastIndex);
147: setStorage.metadata = metadata;
148: setStorage.stamp = DUMMY_STAMP;
149: } else {
150: setStorage.elements[searchIndex].value = lastElement.value;
151:
152: setStorage.firstElement = firstElement;
153: setStorage.numElements = uint8(lastIndex);
154: setStorage.metadata = metadata;
155: setStorage.stamp = DUMMY_STAMP;
156: }
157: } else {
158: setStorage.firstElement = firstElement;
159: setStorage.numElements = uint8(lastIndex);
160: setStorage.metadata = metadata;
161: setStorage.stamp = DUMMY_STAMP;
162: }
163:
164: lastElement.value = address(0);
165:
166: return true;
167: }Contracts should define structs before modifiers and modifiers before functions.
Num of instances: 1
[NonCritical-29] Events may be emitted out of order due to code not follow the best practice of check-effects-interaction
The "check-effects-interaction" pattern also impacts event ordering. When a contract doesn't adhere to this pattern, events might be emitted in a sequence that doesn't reflect the actual logical flow of operations. This can cause confusion during event tracking, potentially leading to erroneous off-chain interpretations. To rectify this, always ensure that checks are performed first, state modifications come next, and interactions with external contracts or addresses are done last. This will ensure events are emitted in a logical, consistent manner, providing a clear and accurate chronological record of on-chain actions for off-chain systems and observers.
Num of instances: 2
Click to show findings
['930']
930: function checkAccountStatusInternal(address account) internal virtual returns (bool isValid, bytes memory result) { // <= FOUND
931: SetStorage storage accountControllersStorage = accountControllers[account];
932: uint256 numOfControllers = accountControllersStorage.numElements;
933: address controller = accountControllersStorage.firstElement;
934: uint8 stamp = accountControllersStorage.stamp;
935:
936: if (numOfControllers == 0) return (true, "");
937: else if (numOfControllers > 1) return (false, abi.encodeWithSelector(EVC_ControllerViolation.selector));
938:
939: bool success;
940: (success, result) =
941: controller.call(abi.encodeCall(IVault.checkAccountStatus, (account, accountCollaterals[account].get()))); // <= FOUND
942:
943: isValid = success && result.length == 32
944: && abi.decode(result, (bytes32)) == bytes32(IVault.checkAccountStatus.selector);
945:
946: if (isValid) {
947: accountControllersStorage.numElements = uint8(numOfControllers);
948: accountControllersStorage.firstElement = controller;
949: accountControllersStorage.metadata = uint80(block.timestamp);
950: accountControllersStorage.stamp = stamp;
951: }
952:
953: emit AccountStatusCheck(account, controller); // <= FOUND
954: }['977']
977: function checkVaultStatusInternal(address vault) internal returns (bool isValid, bytes memory result) { // <= FOUND
978: bool success;
979: (success, result) = vault.call(abi.encodeCall(IVault.checkVaultStatus, ())); // <= FOUND
980:
981: isValid =
982: success && result.length == 32 && abi.decode(result, (bytes32)) == bytes32(IVault.checkVaultStatus.selector);
983:
984: emit VaultStatusCheck(vault); // <= FOUND
985: }Sensitive setter functions in smart contracts often alter critical state variables. Without events emitted in these functions, external observers or dApps cannot easily track or react to these state changes. Missing events can obscure contract activity, hampering transparency and making integration more challenging. To resolve this, incorporate appropriate event emissions within these functions. Events offer an efficient way to log crucial changes, aiding in real-time tracking and post-transaction verification.
Num of instances: 7
Click to show findings
['43']
43: function setOnBehalfOfAccount(EC self, address account) internal pure returns (EC result) { // <= FOUND
44: result = EC.wrap((EC.unwrap(self) & ~ON_BEHALF_OF_ACCOUNT_MASK) | uint160(account));
45: }['51']
51: function setChecksDeferred(EC self) internal pure returns (EC result) { // <= FOUND
52: result = EC.wrap(EC.unwrap(self) | CHECKS_DEFERRED_MASK);
53: }['59']
59: function setChecksInProgress(EC self) internal pure returns (EC result) { // <= FOUND
60: result = EC.wrap(EC.unwrap(self) | CHECKS_IN_PROGRESS_MASK);
61: }['67']
67: function setControlCollateralInProgress(EC self) internal pure returns (EC result) { // <= FOUND
68: result = EC.wrap(EC.unwrap(self) | CONTROL_COLLATERAL_IN_PROGRESS_LOCK_MASK);
69: }['75']
75: function setOperatorAuthenticated(EC self) internal pure returns (EC result) { // <= FOUND
76: result = EC.wrap(EC.unwrap(self) | OPERATOR_AUTHENTICATED_MASK);
77: }['87']
87: function setSimulationInProgress(EC self) internal pure returns (EC result) { // <= FOUND
88: result = EC.wrap(EC.unwrap(self) | SIMULATION_MASK);
89: }['196']
196: function setMetadata(SetStorage storage setStorage, uint80 metadata) internal { // <= FOUND
197: setStorage.metadata = metadata;
198: }EIP-5267 aims to enhance EIP-712, which standardizes Ethereum's structured data signing. While EIP-712 defines how structured data should be signed, EIP-5267 focuses on standardizing the representation of domains. By doing so, it allows applications to fetch domain descriptions and dynamically construct domain separators, aiding in secure EIP-712 signature integration. To implement EIP-5267, contracts should expose a consistent method (e.g., EIP712Domain()) returning the domain's description. Adopting this ensures that applications can universally and securely handle EIP-712 signatures across various contracts, enhancing both security and scalability in Ethereum applications.
Num of instances: 1
Click to show findings
['36']
36: bytes32 internal constant TYPE_HASH =
37: keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); // <= FOUNDFunction parameters in Solidity are passed by value, meaning they are essentially local copies. Mutating them can lead to confusion and errors because the changes don't persist outside the function. By keeping function parameters immutable, you ensure clarity in code behavior, preventing unintended side-effects. If you need to modify a value based on a parameter, use a local variable inside the function, leaving the original parameter unaltered. By adhering to this practice, you maintain a clear distinction between input data and the internal processing logic, improving code readability and reducing the potential for bugs.
Num of instances: 1
Click to show findings
['827']
827: function callWithContextInternal(
828: address targetContract,
829: address onBehalfOfAccount,
830: uint256 value,
831: bytes calldata data
832: ) internal virtual returns (bool success, bytes memory result) {
833: if (value == type(uint256).max) {
834: value = address(this).balance; // <= FOUND
835: } else if (value > address(this).balance) {
836: revert EVC_InvalidValue();
837: }
838:
839: EC contextCache = executionContext;
840: address msgSender = _msgSender();
841:
842:
843:
844:
845:
846:
847:
848:
849: if (
850: haveCommonOwnerInternal(onBehalfOfAccount, msgSender) || targetContract == msg.sender
851: || targetContract == address(this) || contextCache.isControlCollateralInProgress()
852: ) {
853: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).clearOperatorAuthenticated();
854: } else {
855: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).setOperatorAuthenticated();
856: }
857:
858: emit CallWithContext(
859: msgSender, getAddressPrefixInternal(onBehalfOfAccount), onBehalfOfAccount, targetContract, bytes4(data)
860: );
861:
862: (success, result) = targetContract.call{value: value}(data);
863:
864: executionContext = contextCache;
865: }Accepting arrays as parameters in external calls exposes a function to potential Denial-of-Service (DOS) attacks. Attackers can send excessively long arrays to the function, resulting in unanticipatedly high gas consumption. When iterating over such large arrays, a function might consume all the transaction's gas, leading to inadvertent reverts. This behavior can be abused by attackers to effectively block certain functionalities. Resolution: Implement constraints on array length, either by setting a maximum limit or using a dynamic gas check. Always be cautious when iterating over externally-provided arrays to prevent unexpected gas exhaustion and potential DOS vulnerabilities.
Num of instances: 1
Click to show findings
['653']
653: function batchSimulation(BatchItem[] calldata items)
654: external
655: payable
656: virtual
657: returns (
658: BatchItemResult[] memory batchItemsResult,
659: StatusCheckResult[] memory accountsStatusCheckResult,
660: StatusCheckResult[] memory vaultsStatusCheckResult
661: )
662: {
663: (bool success, bytes memory result) = address(this).delegatecall(abi.encodeCall(this.batchRevert, items)); // <= FOUND
664:
665: if (success) {
666: revert EVC_BatchPanic();
667: } else if (result.length < 4 || bytes4(result) != EVC_RevertedBatchResult.selector) {
668: revertBytes(result);
669: }
670:
671: assembly {
672: let length := mload(result)
673:
674: result := add(result, 4)
675:
676:
677: mstore(result, sub(length, 4))
678: }
679:
680: (batchItemsResult, accountsStatusCheckResult, vaultsStatusCheckResult) =
681: abi.decode(result, (BatchItemResult[], StatusCheckResult[], StatusCheckResult[]));
682: }[NonCritical-34] A event should be emitted if a non immutable state variable is set in a constructor
Num of instances: 1
Click to show findings
['27']
27: constructor() {
28:
29:
30: executionContext = EC.wrap(1 << ExecutionContext.STAMP_OFFSET); // <= FOUND
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48: accountStatusChecks.initialize();
49: vaultStatusChecks.initialize();
50: }[NonCritical-35] Use 'using' keyword when using specific imports rather than calling the specific import directly
In Solidity, the using keyword can streamline the use of library functions for specific types. Instead of calling library functions directly with their full import paths, you can declare a library once with using for a specific type. This approach makes your code more readable and concise. For example, instead of LibraryName.functionName(variable), you would first declare using LibraryName for TypeName; at the contract level. After this, you can call library functions directly on variables of TypeName like variable.functionName(). This method not only enhances code clarity but also promotes cleaner and more organized code, especially when multiple functions from the same library are used frequently.
Num of instances: 1
Click to show findings
['30']
30:
31:
32: executionContext = EC.wrap(1 << ExecutionContext.STAMP_OFFSET); // <= FOUND 'ExecutionContext.'Only some address parameters are checked against address(0), to ensure consistency ensure all address parameters are checked.
Num of instances: 1
Click to show findings
['877']
877: function callWithAuthenticationInternal(
878: address targetContract, // <= FOUND 'address targetContract'
879: address onBehalfOfAccount, // <= FOUND 'address onBehalfOfAccount'
880: uint256 value,
881: bytes calldata data
882: ) internal virtual returns (bool success, bytes memory result) {
883: if (targetContract == address(this)) {
884: if (onBehalfOfAccount != address(0)) {
885: revert EVC_InvalidAddress();
886: }
887:
888: if (value != 0) {
889: revert EVC_InvalidValue();
890: }
891:
892:
893: (success, result) = address(this).delegatecall(data);
894: } else {
895:
896:
897: if (targetContract != msg.sender) {
898: authenticateCaller({account: onBehalfOfAccount, allowOperator: true, checkLockdownMode: true});
899: }
900:
901: (success, result) = callWithContextInternal(targetContract, onBehalfOfAccount, value, data);
902: }
903: }Having such variables can create confusion in both developers and in users of the project. Consider renaming these variables to improve code clarity.
Num of instances: 2
Click to show findings
['75']
75:
76: bool isLockdownMode; // <= FOUND['77']
77:
78: bool isPermitDisabledMode; // <= FOUNDEmitting an event in a constructor of a smart contract provides transparency and traceability in blockchain applications. This event logs the contract’s creation, aiding in monitoring and verifying contract deployment. Although constructors are executed only once, the emitted event ensures the contract's initialization is recorded on the blockchain.
Num of instances: 3
Click to show findings
['20']
20: constructor(address _evc) { // <= FOUND
21: if (_evc == address(0)) revert EVC_InvalidAddress();
22:
23: evc = IEVC(_evc);
24: }['94']
94: constructor() { // <= FOUND
95: CACHED_CHAIN_ID = block.chainid;
96: CACHED_DOMAIN_SEPARATOR = calculateDomainSeparator();
97: }['27']
27: constructor() { // <= FOUND
28:
29:
30: executionContext = EC.wrap(1 << ExecutionContext.STAMP_OFFSET);
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48: accountStatusChecks.initialize();
49: vaultStatusChecks.initialize();
50: }Using a fixed compiler version in Solidity contract and abstract files ensures consistency and predictability in smart contract behavior. A fixed version prevents unforeseen issues arising from compiler updates, which might introduce breaking changes or new bugs. It guarantees that the contract's behavior remains stable and consistent with the version used during its development and testing.
Num of instances: 5
Click to show findings
[]
11: contract Errors []
18: contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC []
9: contract Events []
13: abstract contract EVCUtil []
14: abstract contract TransientStorage Emits are designed to make users aware of state variable changes. As such the event declaration should be clear on what it will output, by passing in function calls this can affect the readability of a emit declaration. As such it is advisable to make function calls outside of the event emit and pass the return value into the emit instead.
Num of instances: 1
Click to show findings
['827']
827: function callWithContextInternal(
828: address targetContract,
829: address onBehalfOfAccount,
830: uint256 value,
831: bytes calldata data
832: ) internal virtual returns (bool success, bytes memory result) {
833: if (value == type(uint256).max) {
834: value = address(this).balance;
835: } else if (value > address(this).balance) {
836: revert EVC_InvalidValue();
837: }
838:
839: EC contextCache = executionContext;
840: address msgSender = _msgSender();
841:
842:
843:
844:
845:
846:
847:
848:
849: if (
850: haveCommonOwnerInternal(onBehalfOfAccount, msgSender) || targetContract == msg.sender
851: || targetContract == address(this) || contextCache.isControlCollateralInProgress()
852: ) {
853: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).clearOperatorAuthenticated();
854: } else {
855: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).setOperatorAuthenticated();
856: }
857:
858: emit CallWithContext( // <= FOUND
859: msgSender, getAddressPrefixInternal(onBehalfOfAccount), onBehalfOfAccount, targetContract, bytes4(data)
860: );
861:
862: (success, result) = targetContract.call{value: value}(data);
863:
864: executionContext = contextCache;
865: }In Solidity, custom errors with parameters offer a gas-efficient way to convey detailed information about issues encountered during contract execution. Unlike revert messages, which are strings consuming more gas, custom errors defined with parameters allow developers to specify types and details of errors succinctly. This method enhances debugging, provides clearer insights into contract failures, and improves the developer's and end-user's understanding of what went wrong, all while optimizing for gas usage and maintaining contract efficiency.
Num of instances: 39
Click to show findings
['21']
21: error EVC_InvalidAddress(); // <= FOUND['17']
17: error NotAuthorized(); // <= FOUND['18']
18: error ControllerDisabled(); // <= FOUND['13']
13:
14: error EVC_NotAuthorized(); // <= FOUND['15']
15:
16: error EVC_OnBehalfOfAccountNotAuthenticated(); // <= FOUND['17']
17:
18: error EVC_InvalidOperatorStatus(); // <= FOUND['19']
19:
20: error EVC_InvalidNonce(); // <= FOUND['21']
21:
22: error EVC_InvalidAddress(); // <= FOUND['23']
23:
24: error EVC_InvalidTimestamp(); // <= FOUND['25']
25:
26: error EVC_InvalidValue(); // <= FOUND['27']
27:
28: error EVC_InvalidData(); // <= FOUND['29']
29:
30: error EVC_LockdownMode(); // <= FOUND['31']
31:
32: error EVC_PermitDisabledMode(); // <= FOUND['33']
33:
34: error EVC_ChecksReentrancy(); // <= FOUND['35']
35:
36: error EVC_ControlCollateralReentrancy(); // <= FOUND['37']
37:
38: error EVC_ControllerViolation(); // <= FOUND['39']
39:
40: error EVC_SimulationBatchNested(); // <= FOUND['47']
47:
48: error EVC_BatchPanic(); // <= FOUND['49']
49:
50: error EVC_EmptyError(); // <= FOUND['46']
46: error TooManyElements(); // <= FOUND['47']
47: error InvalidIndex(); // <= FOUND['13']
13: error EVC_NotAuthorized(); // <= FOUND['15']
15: error EVC_OnBehalfOfAccountNotAuthenticated(); // <= FOUND['17']
17: error EVC_InvalidOperatorStatus(); // <= FOUND['19']
19: error EVC_InvalidNonce(); // <= FOUND['21']
21: error EVC_InvalidAddress(); // <= FOUND['23']
23: error EVC_InvalidTimestamp(); // <= FOUND['25']
25: error EVC_InvalidValue(); // <= FOUND['27']
27: error EVC_InvalidData(); // <= FOUND['29']
29: error EVC_LockdownMode(); // <= FOUND['31']
31: error EVC_PermitDisabledMode(); // <= FOUND['33']
33: error EVC_ChecksReentrancy(); // <= FOUND['35']
35: error EVC_ControlCollateralReentrancy(); // <= FOUND['37']
37: error EVC_ControllerViolation(); // <= FOUND['39']
39: error EVC_SimulationBatchNested(); // <= FOUND['47']
47: error EVC_BatchPanic(); // <= FOUND['49']
49: error EVC_EmptyError(); // <= FOUND['17']
17: error NotAuthorized(); // <= FOUND['18']
18: error ControllerDisabled(); // <= FOUND[NonCritical-42] Consider using OpenZeppelins SafeCall library when making calls to arbitrary contracts
Using OpenZeppelin's SafeCall library for interactions with arbitrary contracts is a best practice in smart contract development. This library provides functions that ensure safer external calls by validating that calls are successfully completed, helping to prevent common pitfalls such as reentrancy attacks or unexpected failures. It encapsulates low-level call operations with safety checks, reducing the risk of vulnerabilities associated with direct interactions with unknown code. Incorporating SafeCall enhances contract security and robustness, making it a wise choice for developers aiming to safeguard their applications.
Num of instances: 3
Click to show findings
['827']
827: function callWithContextInternal(
828: address targetContract,
829: address onBehalfOfAccount,
830: uint256 value,
831: bytes calldata data
832: ) internal virtual returns (bool success, bytes memory result) {
833: if (value == type(uint256).max) {
834: value = address(this).balance;
835: } else if (value > address(this).balance) {
836: revert EVC_InvalidValue();
837: }
838:
839: EC contextCache = executionContext;
840: address msgSender = _msgSender();
841:
842:
843:
844:
845:
846:
847:
848:
849: if (
850: haveCommonOwnerInternal(onBehalfOfAccount, msgSender) || targetContract == msg.sender
851: || targetContract == address(this) || contextCache.isControlCollateralInProgress()
852: ) {
853: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).clearOperatorAuthenticated();
854: } else {
855: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).setOperatorAuthenticated();
856: }
857:
858: emit CallWithContext(
859: msgSender, getAddressPrefixInternal(onBehalfOfAccount), onBehalfOfAccount, targetContract, bytes4(data)
860: );
861:
862: (success, result) = targetContract.call{value: value}(data); // <= FOUND
863:
864: executionContext = contextCache;
865: }['930']
930: function checkAccountStatusInternal(address account) internal virtual returns (bool isValid, bytes memory result) { // <= FOUND
931: SetStorage storage accountControllersStorage = accountControllers[account];
932: uint256 numOfControllers = accountControllersStorage.numElements;
933: address controller = accountControllersStorage.firstElement;
934: uint8 stamp = accountControllersStorage.stamp;
935:
936: if (numOfControllers == 0) return (true, "");
937: else if (numOfControllers > 1) return (false, abi.encodeWithSelector(EVC_ControllerViolation.selector));
938:
939: bool success;
940: (success, result) =
941: controller.call(abi.encodeCall(IVault.checkAccountStatus, (account, accountCollaterals[account].get()))); // <= FOUND
942:
943: isValid = success && result.length == 32
944: && abi.decode(result, (bytes32)) == bytes32(IVault.checkAccountStatus.selector);
945:
946: if (isValid) {
947: accountControllersStorage.numElements = uint8(numOfControllers);
948: accountControllersStorage.firstElement = controller;
949: accountControllersStorage.metadata = uint80(block.timestamp);
950: accountControllersStorage.stamp = stamp;
951: }
952:
953: emit AccountStatusCheck(account, controller);
954: }['977']
977: function checkVaultStatusInternal(address vault) internal returns (bool isValid, bytes memory result) { // <= FOUND
978: bool success;
979: (success, result) = vault.call(abi.encodeCall(IVault.checkVaultStatus, ())); // <= FOUND
980:
981: isValid =
982: success && result.length == 32 && abi.decode(result, (bytes32)) == bytes32(IVault.checkVaultStatus.selector);
983:
984: emit VaultStatusCheck(vault);
985: }Tagging assembly blocks as "memory safe" in Solidity helps maintainers and auditors quickly identify areas of the code less likely to introduce vulnerabilities due to improper memory access. This practice promotes safer contract development by clearly communicating assumptions about memory handling, aiding in the review process, and reducing the risk of introducing memory-related bugs. It's a part of best coding practices, enhancing code clarity and security, especially in complex, low-level operations where Solidity's safety checks are bypassed.
Num of instances: 1
Click to show findings
['1176']
1176: assembly {
1177: result := lt(xor(account, otherAccount), 0x100)
1178: }Num of instances: 1
Click to show findings
['196']
196: function setMetadata(SetStorage storage setStorage, uint80 metadata) internal { // <= FOUND
197: setStorage.metadata = metadata; // <= FOUND
198: }Updating a state variable multiple times within a function can lead to inefficiencies and unintended behaviors. Every state change in Ethereum consumes gas, increasing the transaction cost. Moreover, frequent state changes can introduce vulnerabilities if interim values can be externally observed or acted upon. To address this, one should consolidate updates to occur only once, at the end of the function. This minimizes gas usage and ensures consistent state.
Num of instances: 3
Click to show findings
['620']
620: function batchRevert(BatchItem[] calldata items) public payable virtual nonReentrantChecksAndControlCollateral {
621: BatchItemResult[] memory batchItemsResult;
622: StatusCheckResult[] memory accountsStatusCheckResult;
623: StatusCheckResult[] memory vaultsStatusCheckResult;
624:
625: EC contextCache = executionContext;
626:
627: if (contextCache.areChecksDeferred()) {
628: revert EVC_SimulationBatchNested();
629: }
630:
631: executionContext = contextCache.setChecksDeferred().setSimulationInProgress(); // <= FOUND
632:
633: uint256 length = items.length;
634: batchItemsResult = new BatchItemResult[](length);
635:
636: for (uint256 i; i < length; ++i) {
637: BatchItem calldata item = items[i];
638: (batchItemsResult[i].success, batchItemsResult[i].result) =
639: callWithAuthenticationInternal(item.targetContract, item.onBehalfOfAccount, item.value, item.data);
640: }
641:
642: executionContext = contextCache.setChecksInProgress().setOnBehalfOfAccount(address(0)); // <= FOUND
643:
644: accountsStatusCheckResult = checkStatusAllWithResult(SetType.Account);
645: vaultsStatusCheckResult = checkStatusAllWithResult(SetType.Vault);
646:
647: executionContext = contextCache; // <= FOUND
648:
649: revert EVC_RevertedBatchResult(batchItemsResult, accountsStatusCheckResult, vaultsStatusCheckResult);
650: }['827']
827: function callWithContextInternal(
828: address targetContract,
829: address onBehalfOfAccount,
830: uint256 value,
831: bytes calldata data
832: ) internal virtual returns (bool success, bytes memory result) {
833: if (value == type(uint256).max) {
834: value = address(this).balance;
835: } else if (value > address(this).balance) {
836: revert EVC_InvalidValue();
837: }
838:
839: EC contextCache = executionContext;
840: address msgSender = _msgSender();
841:
842:
843:
844:
845:
846:
847:
848:
849: if (
850: haveCommonOwnerInternal(onBehalfOfAccount, msgSender) || targetContract == msg.sender
851: || targetContract == address(this) || contextCache.isControlCollateralInProgress()
852: ) {
853: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).clearOperatorAuthenticated(); // <= FOUND
854: } else {
855: executionContext = contextCache.setOnBehalfOfAccount(onBehalfOfAccount).setOperatorAuthenticated(); // <= FOUND
856: }
857:
858: emit CallWithContext(
859: msgSender, getAddressPrefixInternal(onBehalfOfAccount), onBehalfOfAccount, targetContract, bytes4(data)
860: );
861:
862: (success, result) = targetContract.call{value: value}(data);
863:
864: executionContext = contextCache; // <= FOUND
865: }['911']
911: function restoreExecutionContext(EC contextCache) internal virtual {
912: if (!contextCache.areChecksDeferred()) {
913: executionContext = contextCache.setChecksInProgress().setOnBehalfOfAccount(address(0)); // <= FOUND
914:
915: checkStatusAll(SetType.Account);
916: checkStatusAll(SetType.Vault);
917: }
918:
919: executionContext = contextCache; // <= FOUND
920: }Num of instances: 35
Click to show findings
['28']
28: function EVC() external view returns (address) {
29: return address(evc); // <= FOUND
30: }['78']
78: function _msgSender() internal view virtual returns (address) {
79: address sender = msg.sender;
80:
81: if (sender == address(evc)) {
82: (sender,) = evc.getCurrentOnBehalfOfAccount(address(0));
83: }
84:
85: return sender; // <= FOUND
86: }['93']
93: function _msgSenderForBorrow() internal view virtual returns (address) {
94: address sender = msg.sender;
95: bool controllerEnabled;
96:
97: if (sender == address(evc)) {
98: (sender, controllerEnabled) = evc.getCurrentOnBehalfOfAccount(address(this));
99: } else {
100: controllerEnabled = evc.isControllerEnabled(sender, address(this));
101: }
102:
103: if (!controllerEnabled) {
104: revert ControllerDisabled();
105: }
106:
107: return sender; // <= FOUND
108: }['225']
225: function areChecksDeferred() external view returns (bool) {
226: return executionContext.areChecksDeferred(); // <= FOUND
227: }['230']
230: function areChecksInProgress() external view returns (bool) {
231: return executionContext.areChecksInProgress(); // <= FOUND
232: }['235']
235: function isControlCollateralInProgress() external view returns (bool) {
236: return executionContext.isControlCollateralInProgress(); // <= FOUND
237: }['240']
240: function isOperatorAuthenticated() external view returns (bool) {
241: return executionContext.isOperatorAuthenticated(); // <= FOUND
242: }['245']
245: function isSimulationInProgress() external view returns (bool) {
246: return executionContext.isSimulationInProgress(); // <= FOUND
247: }['262']
262: function getAccountOwner(address account) external view returns (address) {
263: return getAccountOwnerInternal(account); // <= FOUND
264: }['267']
267: function isLockdownMode(bytes19 addressPrefix) external view returns (bool) {
268: return ownerLookup[addressPrefix].isLockdownMode; // <= FOUND
269: }['272']
272: function isPermitDisabledMode(bytes19 addressPrefix) external view returns (bool) {
273: return ownerLookup[addressPrefix].isPermitDisabledMode; // <= FOUND
274: }['277']
277: function getNonce(bytes19 addressPrefix, uint256 nonceNamespace) external view returns (uint256) {
278: return nonceLookup[addressPrefix][nonceNamespace]; // <= FOUND
279: }['282']
282: function getOperator(bytes19 addressPrefix, address operator) external view returns (uint256) {
283: return operatorLookup[addressPrefix][operator]; // <= FOUND
284: }['287']
287: function isAccountOperatorAuthorized(address account, address operator) external view returns (bool) {
288: return isAccountOperatorAuthorizedInternal(account, operator); // <= FOUND
289: }['408']
408: function getCollaterals(address account) external view returns (address[] memory) {
409: return accountCollaterals[account].get(); // <= FOUND
410: }['413']
413: function isCollateralEnabled(address account, address vault) external view returns (bool) {
414: return accountCollaterals[account].contains(vault); // <= FOUND
415: }['454']
454: function getControllers(address account) external view returns (address[] memory) {
455: return accountControllers[account].get(); // <= FOUND
456: }['459']
459: function isControllerEnabled(address account, address vault) external view returns (bool) {
460: return accountControllers[account].contains(vault); // <= FOUND
461: }['687']
687: function getLastAccountStatusCheckTimestamp(address account) external view nonReentrantChecks returns (uint256) {
688: return accountControllers[account].getMetadata(); // <= FOUND
689: }['692']
692: function isAccountStatusCheckDeferred(address account) external view nonReentrantChecks returns (bool) {
693: return accountStatusChecks.contains(account); // <= FOUND
694: }['719']
719: function isVaultStatusCheckDeferred(address vault) external view nonReentrantChecks returns (bool) {
720: return vaultStatusChecks.contains(vault); // <= FOUND
721: }['1134']
1134: function isValidERC1271Signature(
1135: address signer,
1136: bytes32 hash,
1137: bytes memory signature
1138: ) internal view returns (bool isValid) {
1139: if (signer.code.length == 0) return false; // <= FOUND
1140:
1141: (bool success, bytes memory result) =
1142: signer.staticcall(abi.encodeCall(IERC1271.isValidSignature, (hash, signature)));
1143:
1144: isValid = success && result.length == 32
1145: && abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector);
1146: }['1150']
1150: function calculateDomainSeparator() internal view returns (bytes32) {
1151: return keccak256(abi.encode(TYPE_HASH, HASHED_NAME, HASHED_VERSION, block.chainid, address(this))); // <= FOUND
1152: }['1160']
1160: function _msgSender() internal view virtual returns (address) {
1161: return inPermitSelfCall() ? executionContext.getOnBehalfOfAccount() : msg.sender; // <= FOUND
1162: }['1167']
1167: function inPermitSelfCall() internal view returns (bool) {
1168: return address(this) == msg.sender; // <= FOUND
1169: }['1193']
1193: function getAccountOwnerInternal(address account) internal view returns (address) {
1194: bytes19 addressPrefix = getAddressPrefixInternal(account);
1195: return ownerLookup[addressPrefix].owner; // <= FOUND
1196: }['1206']
1206: function isAccountOperatorAuthorizedInternal(
1207: address account,
1208: address operator
1209: ) internal view returns (bool isAuthorized) {
1210: address owner = getAccountOwnerInternal(account);
1211:
1212:
1213: if (owner == address(0)) return false; // <= FOUND
1214:
1215: bytes19 addressPrefix = getAddressPrefixInternal(account);
1216:
1217:
1218:
1219:
1220: uint256 bitMask = 1 << (uint160(owner) ^ uint160(account));
1221:
1222: return operatorLookup[addressPrefix][operator] & bitMask != 0; // <= FOUND
1223: }['204']
204: function get(SetStorage storage setStorage) internal view returns (address[] memory) {
205: address firstElement = setStorage.firstElement;
206: uint256 numElements = setStorage.numElements;
207: address[] memory output = new address[](numElements);
208:
209: if (numElements == 0) return output; // <= FOUND
210:
211: output[0] = firstElement;
212:
213: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
214: output[i] = setStorage.elements[i].value;
215: }
216:
217: return output; // <= FOUND
218: }['223']
223: function getMetadata(SetStorage storage setStorage) internal view returns (uint80) {
224: return setStorage.metadata; // <= FOUND
225: }['232']
232: function contains(SetStorage storage setStorage, address element) internal view returns (bool) {
233: address firstElement = setStorage.firstElement;
234: uint256 numElements = setStorage.numElements;
235:
236: if (numElements == 0) return false; // <= FOUND
237: if (firstElement == element) return true; // <= FOUND
238:
239: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
240: if (setStorage.elements[i].value == element) return true; // <= FOUND
241: }
242:
243: return false; // <= FOUND
244: }['252']
252: function haveCommonOwner(address account, address otherAccount) external pure returns (bool) {
253: return haveCommonOwnerInternal(account, otherAccount); // <= FOUND
254: }['257']
257: function getAddressPrefix(address account) external pure returns (bytes19) {
258: return getAddressPrefixInternal(account); // <= FOUND
259: }['1041']
1041: function isSignerValid(address signer) internal pure virtual returns (bool) {
1042:
1043:
1044: return !haveCommonOwnerInternal(signer, address(0)); // <= FOUND
1045: }['1091']
1091: function recoverECDSASigner(bytes32 hash, bytes memory signature) internal pure returns (address signer) {
1092: if (signature.length != 65) return address(0); // <= FOUND
1093:
1094: bytes32 r;
1095: bytes32 s;
1096: uint8 v;
1097:
1098:
1099:
1100:
1101: assembly {
1102: r := mload(add(signature, 0x20))
1103: s := mload(add(signature, 0x40))
1104: v := byte(0, mload(add(signature, 0x60)))
1105: }
1106:
1107:
1108:
1109:
1110:
1111:
1112:
1113:
1114:
1115:
1116: if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) {
1117: return address(0); // <= FOUND
1118: }
1119:
1120:
1121: signer = ecrecover(hash, v, r, s);
1122: }['1186']
1186: function getAddressPrefixInternal(address account) internal pure returns (bytes19) {
1187: return bytes19(uint152(uint160(account) >> ACCOUNT_ID_OFFSET)); // <= FOUND
1188: }When using a smaller int/uint type it first needs to be converted to it's 258 bit counterpart to be operated, this increases the gass cost and thus should be avoided. However it does make sense to use smaller int/uint values within structs provided you pack the struct properly.
Num of instances: 12
Click to show findings
['442']
442: function reorderCollaterals(
443: address account,
444: uint8 index1, // <= FOUND
445: uint8 index2 // <= FOUND
446: ) public payable virtual nonReentrantChecksAndControlCollateral onlyOwnerOrOperator(account) {
447: accountCollaterals[account].reorder(index1, index2);
448: requireAccountStatusCheck(account);
449: }['930']
930: function checkAccountStatusInternal(address account) internal virtual returns (bool isValid, bytes memory result) {
931: SetStorage storage accountControllersStorage = accountControllers[account];
932: uint256 numOfControllers = accountControllersStorage.numElements;
933: address controller = accountControllersStorage.firstElement;
934: uint8 stamp = accountControllersStorage.stamp; // <= FOUND
935:
936: if (numOfControllers == 0) return (true, "");
937: else if (numOfControllers > 1) return (false, abi.encodeWithSelector(EVC_ControllerViolation.selector));
938:
939: bool success;
940: (success, result) =
941: controller.call(abi.encodeCall(IVault.checkAccountStatus, (account, accountCollaterals[account].get())));
942:
943: isValid = success && result.length == 32
944: && abi.decode(result, (bytes32)) == bytes32(IVault.checkAccountStatus.selector);
945:
946: if (isValid) {
947: accountControllersStorage.numElements = uint8(numOfControllers);
948: accountControllersStorage.firstElement = controller;
949: accountControllersStorage.metadata = uint80(block.timestamp);
950: accountControllersStorage.stamp = stamp;
951: }
952:
953: emit AccountStatusCheck(account, controller);
954: }['1091']
1091: function recoverECDSASigner(bytes32 hash, bytes memory signature) internal pure returns (address signer) {
1092: if (signature.length != 65) return address(0);
1093:
1094: bytes32 r;
1095: bytes32 s;
1096: uint8 v; // <= FOUND
1097:
1098:
1099:
1100:
1101: assembly {
1102: r := mload(add(signature, 0x20))
1103: s := mload(add(signature, 0x40))
1104: v := byte(0, mload(add(signature, 0x60)))
1105: }
1106:
1107:
1108:
1109:
1110:
1111:
1112:
1113:
1114:
1115:
1116: if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) {
1117: return address(0);
1118: }
1119:
1120:
1121: signer = ecrecover(hash, v, r, s);
1122: }['176']
176: function reorder(SetStorage storage setStorage, uint8 index1, uint8 index2) internal { // <= FOUND
177: address firstElement = setStorage.firstElement;
178: uint256 numElements = setStorage.numElements;
179:
180: if (index1 >= index2 || index2 >= numElements) {
181: revert InvalidIndex();
182: }
183:
184: if (index1 == 0) {
185: (setStorage.firstElement, setStorage.elements[index2].value) =
186: (setStorage.elements[index2].value, firstElement);
187: } else {
188: (setStorage.elements[index1].value, setStorage.elements[index2].value) =
189: (setStorage.elements[index2].value, setStorage.elements[index1].value);
190: }
191: }['49']
49: uint8 internal constant EMPTY_ELEMENT_OFFSET = 1; // <= FOUND['50']
50: uint8 internal constant DUMMY_STAMP = 1; // <= FOUND['71']
71: function insert(SetStorage storage setStorage, address element) internal returns (bool) {
72: address firstElement = setStorage.firstElement;
73: uint256 numElements = setStorage.numElements;
74: uint80 metadata = setStorage.metadata; // <= FOUND
75:
76: if (numElements == 0) {
77:
78:
79:
80: setStorage.numElements = 1;
81: setStorage.firstElement = element;
82: setStorage.metadata = metadata;
83: setStorage.stamp = DUMMY_STAMP;
84: return true;
85: }
86:
87: if (firstElement == element) return false;
88:
89: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
90: if (setStorage.elements[i].value == element) return false;
91: }
92:
93: if (numElements == SET_MAX_ELEMENTS) revert TooManyElements();
94:
95: setStorage.elements[numElements].value = element;
96:
97: unchecked {
98: setStorage.numElements = uint8(numElements + 1);
99: }
100:
101: return true;
102: }['110']
110: function remove(SetStorage storage setStorage, address element) internal returns (bool) {
111: address firstElement = setStorage.firstElement;
112: uint256 numElements = setStorage.numElements;
113: uint80 metadata = setStorage.metadata; // <= FOUND
114:
115: if (numElements == 0) return false;
116:
117: uint256 searchIndex;
118: if (firstElement != element) {
119: for (searchIndex = EMPTY_ELEMENT_OFFSET; searchIndex < numElements; ++searchIndex) {
120: if (setStorage.elements[searchIndex].value == element) break;
121: }
122:
123: if (searchIndex == numElements) return false;
124: }
125:
126:
127: if (numElements == 1) {
128: setStorage.numElements = 0;
129: setStorage.firstElement = address(0);
130: setStorage.metadata = metadata;
131: setStorage.stamp = DUMMY_STAMP;
132: return true;
133: }
134:
135: uint256 lastIndex;
136: unchecked {
137: lastIndex = numElements - 1;
138: }
139:
140:
141:
142: ElementStorage storage lastElement = setStorage.elements[lastIndex];
143: if (searchIndex != lastIndex) {
144: if (searchIndex == 0) {
145: setStorage.firstElement = lastElement.value;
146: setStorage.numElements = uint8(lastIndex);
147: setStorage.metadata = metadata;
148: setStorage.stamp = DUMMY_STAMP;
149: } else {
150: setStorage.elements[searchIndex].value = lastElement.value;
151:
152: setStorage.firstElement = firstElement;
153: setStorage.numElements = uint8(lastIndex);
154: setStorage.metadata = metadata;
155: setStorage.stamp = DUMMY_STAMP;
156: }
157: } else {
158: setStorage.firstElement = firstElement;
159: setStorage.numElements = uint8(lastIndex);
160: setStorage.metadata = metadata;
161: setStorage.stamp = DUMMY_STAMP;
162: }
163:
164: lastElement.value = address(0);
165:
166: return true;
167: }['196']
196: function setMetadata(SetStorage storage setStorage, uint80 metadata) internal { // <= FOUND
197: setStorage.metadata = metadata;
198: }['251']
251: function forEachAndClear(SetStorage storage setStorage, function(address) callback) internal {
252: uint256 numElements = setStorage.numElements;
253: address firstElement = setStorage.firstElement;
254: uint80 metadata = setStorage.metadata; // <= FOUND
255:
256: if (numElements == 0) return;
257:
258: setStorage.numElements = 0;
259: setStorage.firstElement = address(0);
260: setStorage.metadata = metadata;
261: setStorage.stamp = DUMMY_STAMP;
262:
263: callback(firstElement);
264:
265: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
266: address element = setStorage.elements[i].value;
267: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP});
268:
269: callback(element);
270: }
271: }['281']
281: function forEachAndClearWithResult(
282: SetStorage storage setStorage,
283: function(address) returns (bool, bytes memory) callback
284: ) internal returns (bytes[] memory) {
285: uint256 numElements = setStorage.numElements;
286: address firstElement = setStorage.firstElement;
287: uint80 metadata = setStorage.metadata; // <= FOUND
288: bytes[] memory results = new bytes[](numElements);
289:
290: if (numElements == 0) return results;
291:
292: setStorage.numElements = 0;
293: setStorage.firstElement = address(0);
294: setStorage.metadata = metadata;
295: setStorage.stamp = DUMMY_STAMP;
296:
297: (bool success, bytes memory result) = callback(firstElement);
298: results[0] = abi.encode(firstElement, success, result);
299:
300: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
301: address element = setStorage.elements[i].value;
302: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP});
303:
304: (success, result) = callback(element);
305: results[i] = abi.encode(element, success, result);
306: }
307:
308: return results;
309: }['32']
32: uint160 internal constant ACCOUNT_ID_OFFSET = 8; // <= FOUNDReplace spotted instances with != 0 for uints as this uses less gas
Num of instances: 1
Click to show findings
['1116']
1116:
1117:
1118:
1119:
1120:
1121:
1122:
1123:
1124:
1125: if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) { // <= FOUNDUsing .delete is better than resetting a Solidity variable to its default value manually because it frees up storage space on the Ethereum blockchain, resulting in gas cost savings.
Num of instances: 2
Click to show findings
['128']
128: setStorage.numElements = 0; // <= FOUND['128']
128: setStorage.numElements = 0; // <= FOUNDMaking function calls or external calls within loops in Solidity can lead to inefficient gas usage, potential bottlenecks, and increased vulnerability to attacks. Each function call or external call consumes gas, and when executed within a loop, the gas cost multiplies, potentially causing the transaction to run out of gas or exceed block gas limits. This can result in transaction failure or unpredictable behavior.
Num of instances: 3
Click to show findings
['606']
606: for (uint256 i; i < length; ++i) {
607: BatchItem calldata item = items[i];
608: (bool success, bytes memory result) =
609: callWithAuthenticationInternal(item.targetContract, item.onBehalfOfAccount, item.value, item.data); // <= FOUND
610:
611: if (!success) revertBytes(result); // <= FOUND
612: }['606']
606: for (uint256 i; i < length; ++i) {
607: BatchItem calldata item = items[i];
608: (bool success, bytes memory result) =
609: callWithAuthenticationInternal(item.targetContract, item.onBehalfOfAccount, item.value, item.data); // <= FOUND
610:
611: if (!success) revertBytes(result); // <= FOUND
612: }['636']
636: for (uint256 i; i < length; ++i) {
637: BatchItem calldata item = items[i];
638: (batchItemsResult[i].success, batchItemsResult[i].result) =
639: callWithAuthenticationInternal(item.targetContract, item.onBehalfOfAccount, item.value, item.data); // <= FOUND
640: }[Gas-8] For loops in public or external functions should be avoided due to high gas costs and possible DOS
In Solidity, for loops can potentially cause Denial of Service (DoS) attacks if not handled carefully. DoS attacks can occur when an attacker intentionally exploits the gas cost of a function, causing it to run out of gas or making it too expensive for other users to call. Below are some scenarios where for loops can lead to DoS attacks: Nested for loops can become exceptionally gas expensive and should be used sparingly
Num of instances: 2
Click to show findings
['601']
601: function batch(BatchItem[] calldata items) public payable virtual nonReentrantChecksAndControlCollateral {
602: EC contextCache = executionContext;
603: executionContext = contextCache.setChecksDeferred();
604:
605: uint256 length = items.length;
606: for (uint256 i; i < length; ++i) { // <= FOUND
607: BatchItem calldata item = items[i];
608: (bool success, bytes memory result) =
609: callWithAuthenticationInternal(item.targetContract, item.onBehalfOfAccount, item.value, item.data);
610:
611: if (!success) revertBytes(result);
612: }
613:
614: restoreExecutionContext(contextCache);
615: }['620']
620: function batchRevert(BatchItem[] calldata items) public payable virtual nonReentrantChecksAndControlCollateral {
621: BatchItemResult[] memory batchItemsResult;
622: StatusCheckResult[] memory accountsStatusCheckResult;
623: StatusCheckResult[] memory vaultsStatusCheckResult;
624:
625: EC contextCache = executionContext;
626:
627: if (contextCache.areChecksDeferred()) {
628: revert EVC_SimulationBatchNested();
629: }
630:
631: executionContext = contextCache.setChecksDeferred().setSimulationInProgress();
632:
633: uint256 length = items.length;
634: batchItemsResult = new BatchItemResult[](length);
635:
636: for (uint256 i; i < length; ++i) { // <= FOUND
637: BatchItem calldata item = items[i];
638: (batchItemsResult[i].success, batchItemsResult[i].result) =
639: callWithAuthenticationInternal(item.targetContract, item.onBehalfOfAccount, item.value, item.data);
640: }
641:
642: executionContext = contextCache.setChecksInProgress().setOnBehalfOfAccount(address(0));
643:
644: accountsStatusCheckResult = checkStatusAllWithResult(SetType.Account);
645: vaultsStatusCheckResult = checkStatusAllWithResult(SetType.Vault);
646:
647: executionContext = contextCache;
648:
649: revert EVC_RevertedBatchResult(batchItemsResult, accountsStatusCheckResult, vaultsStatusCheckResult);
650: }Using assembly for address comparisons in Solidity can save gas because it allows for more direct access to the Ethereum Virtual Machine (EVM), reducing the overhead of higher-level operations. Solidity's high-level abstraction simplifies coding but can introduce additional gas costs. Using assembly for simple operations like address comparisons can be more gas-efficient.
Num of instances: 8
Click to show findings
['208']
208: function getCurrentOnBehalfOfAccount(address controllerToCheck)
209: external
210: view
211: returns (address onBehalfOfAccount, bool controllerEnabled)
212: {
213: onBehalfOfAccount = executionContext.getOnBehalfOfAccount();
214:
215:
216: if (onBehalfOfAccount == address(0)) { // <= FOUND
217: revert EVC_OnBehalfOfAccountNotAuthenticated();
218: }
219:
220: controllerEnabled =
221: controllerToCheck == address(0) ? false : accountControllers[onBehalfOfAccount].contains(controllerToCheck); // <= FOUND
222: }['487']
487: function permit(
488: address signer,
489: address sender,
490: uint256 nonceNamespace,
491: uint256 nonce,
492: uint256 deadline,
493: uint256 value,
494: bytes calldata data,
495: bytes calldata signature
496: ) public payable virtual nonReentrantChecksAndControlCollateral {
497:
498:
499: if (inPermitSelfCall() || (sender != address(0) && sender != msg.sender)) { // <= FOUND
500: revert EVC_NotAuthorized();
501: }
502:
503: if (signer == address(0) || !isSignerValid(signer)) { // <= FOUND
504: revert EVC_InvalidAddress();
505: }
506:
507: bytes19 addressPrefix = getAddressPrefixInternal(signer);
508:
509: if (ownerLookup[addressPrefix].isPermitDisabledMode) {
510: revert EVC_PermitDisabledMode();
511: }
512:
513: {
514: uint256 currentNonce = nonceLookup[addressPrefix][nonceNamespace];
515:
516: if (currentNonce == type(uint256).max || currentNonce != nonce) {
517: revert EVC_InvalidNonce();
518: }
519: }
520:
521: if (deadline < block.timestamp) {
522: revert EVC_InvalidTimestamp();
523: }
524:
525: if (data.length == 0) {
526: revert EVC_InvalidData();
527: }
528:
529: bytes32 permitHash = getPermitHash(signer, sender, nonceNamespace, nonce, deadline, value, data);
530:
531: if (
532: signer != recoverECDSASigner(permitHash, signature)
533: && !isValidERC1271Signature(signer, permitHash, signature)
534: ) {
535: revert EVC_NotAuthorized();
536: }
537:
538: unchecked {
539: nonceLookup[addressPrefix][nonceNamespace] = nonce + 1;
540: }
541:
542: emit NonceUsed(addressPrefix, nonceNamespace, nonce);
543:
544:
545:
546: (bool success, bytes memory result) = callWithContextInternal(address(this), signer, value, data);
547:
548: if (!success) revertBytes(result);
549: }['759']
759: function authenticateCaller(
760: address account,
761: bool allowOperator,
762: bool checkLockdownMode
763: ) internal virtual returns (address) {
764: bytes19 addressPrefix = getAddressPrefixInternal(account);
765: address owner = ownerLookup[addressPrefix].owner;
766: bool lockdownMode = ownerLookup[addressPrefix].isLockdownMode;
767: address msgSender = _msgSender();
768: bool authenticated = false;
769:
770:
771: if (haveCommonOwnerInternal(account, msgSender)) {
772:
773: if (owner == address(0)) { // <= FOUND
774: ownerLookup[addressPrefix].owner = msgSender;
775: emit OwnerRegistered(addressPrefix, msgSender);
776: authenticated = true;
777: } else if (owner == msgSender) {
778: authenticated = true;
779: }
780: }
781:
782:
783: if (!authenticated && allowOperator && isAccountOperatorAuthorizedInternal(account, msgSender)) {
784: authenticated = true;
785: }
786:
787:
788: if (!authenticated) {
789: revert EVC_NotAuthorized();
790: }
791:
792:
793: if (checkLockdownMode && lockdownMode) {
794: revert EVC_LockdownMode();
795: }
796:
797: return msgSender;
798: }['877']
877: function callWithAuthenticationInternal(
878: address targetContract,
879: address onBehalfOfAccount,
880: uint256 value,
881: bytes calldata data
882: ) internal virtual returns (bool success, bytes memory result) {
883: if (targetContract == address(this)) {
884: if (onBehalfOfAccount != address(0)) { // <= FOUND
885: revert EVC_InvalidAddress();
886: }
887:
888: if (value != 0) {
889: revert EVC_InvalidValue();
890: }
891:
892:
893: (success, result) = address(this).delegatecall(data);
894: } else {
895:
896:
897: if (targetContract != msg.sender) {
898: authenticateCaller({account: onBehalfOfAccount, allowOperator: true, checkLockdownMode: true});
899: }
900:
901: (success, result) = callWithContextInternal(targetContract, onBehalfOfAccount, value, data);
902: }
903: }['1206']
1206: function isAccountOperatorAuthorizedInternal(
1207: address account,
1208: address operator
1209: ) internal view returns (bool isAuthorized) {
1210: address owner = getAccountOwnerInternal(account);
1211:
1212:
1213: if (owner == address(0)) return false; // <= FOUND
1214:
1215: bytes19 addressPrefix = getAddressPrefixInternal(account);
1216:
1217:
1218:
1219:
1220: uint256 bitMask = 1 << (uint160(owner) ^ uint160(account));
1221:
1222: return operatorLookup[addressPrefix][operator] & bitMask != 0;
1223: }['110']
110: function remove(SetStorage storage setStorage, address element) internal returns (bool) {
111: address firstElement = setStorage.firstElement;
112: uint256 numElements = setStorage.numElements;
113: uint80 metadata = setStorage.metadata;
114:
115: if (numElements == 0) return false;
116:
117: uint256 searchIndex;
118: if (firstElement != element) {
119: for (searchIndex = EMPTY_ELEMENT_OFFSET; searchIndex < numElements; ++searchIndex) {
120: if (setStorage.elements[searchIndex].value == element) break;
121: }
122:
123: if (searchIndex == numElements) return false;
124: }
125:
126:
127: if (numElements == 1) {
128: setStorage.numElements = 0;
129: setStorage.firstElement = address(0); // <= FOUND
130: setStorage.metadata = metadata;
131: setStorage.stamp = DUMMY_STAMP;
132: return true;
133: }
134:
135: uint256 lastIndex;
136: unchecked {
137: lastIndex = numElements - 1;
138: }
139:
140:
141:
142: ElementStorage storage lastElement = setStorage.elements[lastIndex];
143: if (searchIndex != lastIndex) {
144: if (searchIndex == 0) {
145: setStorage.firstElement = lastElement.value;
146: setStorage.numElements = uint8(lastIndex);
147: setStorage.metadata = metadata;
148: setStorage.stamp = DUMMY_STAMP;
149: } else {
150: setStorage.elements[searchIndex].value = lastElement.value;
151:
152: setStorage.firstElement = firstElement;
153: setStorage.numElements = uint8(lastIndex);
154: setStorage.metadata = metadata;
155: setStorage.stamp = DUMMY_STAMP;
156: }
157: } else {
158: setStorage.firstElement = firstElement;
159: setStorage.numElements = uint8(lastIndex);
160: setStorage.metadata = metadata;
161: setStorage.stamp = DUMMY_STAMP;
162: }
163:
164: lastElement.value = address(0); // <= FOUND
165:
166: return true;
167: }['251']
251: function forEachAndClear(SetStorage storage setStorage, function(address) callback) internal {
252: uint256 numElements = setStorage.numElements;
253: address firstElement = setStorage.firstElement;
254: uint80 metadata = setStorage.metadata;
255:
256: if (numElements == 0) return;
257:
258: setStorage.numElements = 0;
259: setStorage.firstElement = address(0); // <= FOUND
260: setStorage.metadata = metadata;
261: setStorage.stamp = DUMMY_STAMP;
262:
263: callback(firstElement);
264:
265: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
266: address element = setStorage.elements[i].value;
267: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP});
268:
269: callback(element);
270: }
271: }['281']
281: function forEachAndClearWithResult(
282: SetStorage storage setStorage,
283: function(address) returns (bool, bytes memory) callback
284: ) internal returns (bytes[] memory) {
285: uint256 numElements = setStorage.numElements;
286: address firstElement = setStorage.firstElement;
287: uint80 metadata = setStorage.metadata;
288: bytes[] memory results = new bytes[](numElements);
289:
290: if (numElements == 0) return results;
291:
292: setStorage.numElements = 0;
293: setStorage.firstElement = address(0); // <= FOUND
294: setStorage.metadata = metadata;
295: setStorage.stamp = DUMMY_STAMP;
296:
297: (bool success, bytes memory result) = callback(firstElement);
298: results[0] = abi.encode(firstElement, success, result);
299:
300: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
301: address element = setStorage.elements[i].value;
302: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP});
303:
304: (success, result) = callback(element);
305: results[i] = abi.encode(element, success, result);
306: }
307:
308: return results;
309: }From a gas efficiency perspective, using _msgSender() in a contract not intended to support EIP-2771 could add unnecessary overhead. The _msgSender() function includes checks to determine if the transaction was forwarded, which involves extra function calls that consume more gas than a simple msg.sender.
If a contract doesn't require EIP-2771 meta-transaction support, using msg.sender directly is more gas efficient. msg.sender is a globally accessible variable in Solidity that doesn't require an extra function call, making it a less costly choice.
In the context of Ethereum, where every operation has a gas cost, it's crucial to eliminate unnecessary computations to optimize contract execution and minimize transaction fees. Therefore, if EIP-2771 support isn't necessary, it's recommended to use msg.sender instead of _msgSender().
Num of instances: 3
Click to show findings
['78']
78:
82: function _msgSender() internal view virtual returns (address) { // <= FOUND['767']
767: address msgSender = _msgSender(); // <= FOUND['78']
78:
84: function _msgSender() internal view virtual returns (address) { // <= FOUNDBitmaps in Solidity are essentially a way of representing a set of boolean values within an integer type variable such as uint256. Each bit in the integer represents a true or false value (1 or 0), thus allowing efficient storage of multiple boolean values.
Bitmaps can save gas in the Ethereum network because they condense a lot of information into a small amount of storage. In Ethereum, storage is one of the most significant costs in terms of gas usage. By reducing the amount of storage space needed, you can potentially save on gas fees.
Here's a quick comparison:
If you were to represent 256 different boolean values in the traditional way, you would have to declare 256 different bool variables. Given that each bool occupies a storage slot and each storage slot costs 20,000 gas to initialize, you would end up paying a considerable amount of gas.
On the other hand, if you were to use a bitmap, you could store these 256 boolean values within a single uint256 variable. In other words, you'd only pay for a single storage slot, resulting in significant gas savings.
However, it's important to note that while bitmaps can provide gas efficiencies, they do add complexity to the code, making it harder to read and maintain. Also, using bitmaps is efficient only when dealing with a large number of boolean variables that are frequently changed or accessed together.
In contrast, the straightforward counterpart to bitmaps would be using arrays or mappings to store boolean values, with each bool value occupying its own storage slot. This approach is simpler and more readable but could potentially be more expensive in terms of gas usage.
Num of instances: 3
Click to show findings
['776']
776: authenticated = true; // <= FOUND['776']
776: authenticated = true; // <= FOUND['768']
768: bool authenticated = false; // <= FOUNDNested mappings and multi-dimensional arrays in Solidity operate through a process of double hashing, wherein the original storage slot and the first key are concatenated and hashed, and then this hash is again concatenated with the second key and hashed. This process can be quite gas expensive due to the double-hashing operation and subsequent storage operation (sstore).
A possible optimization involves manually concatenating the keys followed by a single hash operation and an sstore. However, this technique introduces the risk of storage collision, especially when there are other nested hash maps in the contract that use the same key types. Because Solidity is unaware of the number and structure of nested hash maps in a contract, it follows a conservative approach in computing the storage slot to avoid possible collisions.
OpenZeppelin's EnumerableSet provides a potential solution to this problem. It creates a data structure that combines the benefits of set operations with the ability to enumerate stored elements, which is not natively available in Solidity. EnumerableSet handles the element uniqueness internally and can therefore provide a more gas-efficient and collision-resistant alternative to nested mappings or multi-dimensional arrays in certain scenarios.
Num of instances: 2
Click to show findings
['82']
82: mapping(bytes19 addressPrefix => mapping(address operator => uint256 operatorBitField)) internal operatorLookup; // <= FOUND['84']
84: mapping(bytes19 addressPrefix => mapping(uint256 nonceNamespace => uint256 nonce)) internal nonceLookup; // <= FOUNDIn Solidity, using selfBalance instead of address(this).balance is advantageous for gas optimization. address(this).balance performs an external call to retrieve the contract's balance, which costs extra gas. As a resolution, defining a selfBalance state variable that's updated accordingly provides a more gas-efficient approach. Using selfBalance instead of address(this).balance can save around 800 gas per call. This is because address(this).balance makes an EXTBAL operation, which costs 700 gas, and additionally, the operation is a SLOAD, which costs around 800 gas. Please note that these estimates are based on the Ethereum gas schedule and might vary depending on the Ethereum network state and its future upgrades.
Num of instances: 2
Click to show findings
['834']
834: value = address(this).balance; // <= FOUND['835']
835: } else if (value > address(this).balance) { // <= FOUNDWith the use of inline assembly in Solidity, we can take advantage of low-level features like scratch space and the free memory pointer, offering more gas-efficient ways of emitting events. The scratch space is a certain area of memory where we can temporarily store data, and the free memory pointer indicates the next available memory slot. Using these, we can efficiently assemble event data without incurring additional memory expansion costs. However, safety is paramount: to avoid overwriting or leakage, we must cache the free memory pointer before use and restore it afterward, ensuring that it points to the correct memory location post-operation.
Num of instances: 14
Click to show findings
['302']
302: emit LockdownModeStatus(addressPrefix, enabled); // <= FOUND['320']
320: emit PermitDisabledModeStatus(addressPrefix, enabled); // <= FOUND['338']
338: emit NonceStatus(addressPrefix, nonceNamespace, currentNonce, nonce); // <= FOUND['358']
358: emit OperatorStatus(addressPrefix, operator, operatorBitField); // <= FOUND['401']
401: emit OperatorStatus(addressPrefix, operator, newOperatorBitField); // <= FOUND['425']
425: emit CollateralStatus(account, vault, true); // <= FOUND['436']
436: emit CollateralStatus(account, vault, false); // <= FOUND['471']
471: emit ControllerStatus(account, vault, true); // <= FOUND['479']
479: emit ControllerStatus(account, msg.sender, false); // <= FOUND['542']
542: emit NonceUsed(addressPrefix, nonceNamespace, nonce); // <= FOUND['775']
775: emit OwnerRegistered(addressPrefix, msgSender); // <= FOUND['858']
858: emit CallWithContext( // <= FOUND
859: msgSender, getAddressPrefixInternal(onBehalfOfAccount), onBehalfOfAccount, targetContract, bytes4(data)
860: );['953']
953: emit AccountStatusCheck(account, controller); // <= FOUND['984']
984: emit VaultStatusCheck(vault); // <= FOUNDUsing inline assembly to extract calldata values can be more gas-efficient than using abi.decode in Solidity. Inline assembly gives more direct access to EVM operations, enabling optimized usage of calldata. However, assembly should be used judiciously as it's more prone to errors. Opt for this approach when performance is critical and the complexity it introduces is manageable.
Num of instances: 5
Click to show findings
['680']
680: (batchItemsResult, accountsStatusCheckResult, vaultsStatusCheckResult) =
681: abi.decode(result, (BatchItemResult[], StatusCheckResult[], StatusCheckResult[])); // <= FOUND['943']
943: isValid = success && result.length == 32
944: && abi.decode(result, (bytes32)) == bytes32(IVault.checkAccountStatus.selector); // <= FOUND['981']
981: isValid =
982: success && result.length == 32 && abi.decode(result, (bytes32)) == bytes32(IVault.checkVaultStatus.selector); // <= FOUND['1027']
1027: (address checkedAddress, bool isValid, bytes memory result) =
1028: abi.decode(callbackResult[i], (address, bool, bytes)); // <= FOUND['1144']
1144: isValid = success && result.length == 32
1145: && abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector); // <= FOUNDUsing private visibility for constants and immutables in Solidity instead of public can save gas. This is because private elements are not included in the contract's ABI, reducing the deployment and interaction costs. To achieve better efficiency, it is recommended to use private visibility when external access is not needed.
Num of instances: 1
Click to show findings
['27']
27: string public constant name = "Ethereum Vault Connector"; // <= FOUNDIn Solidity, the unchecked block allows arithmetic operations to not revert on overflow. Without using unchecked in loops, extra gas is consumed due to overflow checks. If it's certain that overflows won't occur within the loop, using unchecked can make the loop more gas-efficient by skipping unnecessary checks.
Num of instances: 17
Click to show findings
['606']
606: for (uint256 i; i < length; ++i) {
607: BatchItem calldata item = items[i];
608: (bool success, bytes memory result) =
609: callWithAuthenticationInternal(item.targetContract, item.onBehalfOfAccount, item.value, item.data);
610:
611: if (!success) revertBytes(result);
612: }['1026']
1026: for (uint256 i; i < length; ++i) {
1027: (address checkedAddress, bool isValid, bytes memory result) =
1028: abi.decode(callbackResult[i], (address, bool, bytes));
1029: checksResult[i] = StatusCheckResult({checkedAddress: checkedAddress, isValid: isValid, result: result});
1030: }['60']
60: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < SET_MAX_ELEMENTS; ++i) {
61: setStorage.elements[i].stamp = DUMMY_STAMP;
62: }['213']
213: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
214: output[i] = setStorage.elements[i].value;
215: }['239']
239: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
240: if (setStorage.elements[i].value == element) return true;
241: }['265']
265: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
266: address element = setStorage.elements[i].value;
267: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP});
268:
269: callback(element);
270: }['300']
300: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
301: address element = setStorage.elements[i].value;
302: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP});
303:
304: (success, result) = callback(element);
305: results[i] = abi.encode(element, success, result);
306: }['606']
606: for (uint256 i; i < length; ++i) {
607: BatchItem calldata item = items[i];
608: (bool success, bytes memory result) =
609: callWithAuthenticationInternal(item.targetContract, item.onBehalfOfAccount, item.value, item.data);
610:
611: if (!success) revertBytes(result);
612: }['636']
636: for (uint256 i; i < length; ++i) {
637: BatchItem calldata item = items[i];
638: (batchItemsResult[i].success, batchItemsResult[i].result) =
639: callWithAuthenticationInternal(item.targetContract, item.onBehalfOfAccount, item.value, item.data);
640: }['1026']
1026: for (uint256 i; i < length; ++i) {
1027: (address checkedAddress, bool isValid, bytes memory result) =
1028: abi.decode(callbackResult[i], (address, bool, bytes));
1029: checksResult[i] = StatusCheckResult({checkedAddress: checkedAddress, isValid: isValid, result: result});
1030: }['60']
60: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < SET_MAX_ELEMENTS; ++i) {
61: setStorage.elements[i].stamp = DUMMY_STAMP;
62: }['89']
89: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
90: if (setStorage.elements[i].value == element) return false;
91: }['119']
119: for (searchIndex = EMPTY_ELEMENT_OFFSET; searchIndex < numElements; ++searchIndex) {
120: if (setStorage.elements[searchIndex].value == element) break;
121: }['213']
213: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
214: output[i] = setStorage.elements[i].value;
215: }['239']
239: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
240: if (setStorage.elements[i].value == element) return true;
241: }['265']
265: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
266: address element = setStorage.elements[i].value;
267: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP});
268:
269: callback(element);
270: }['300']
300: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
301: address element = setStorage.elements[i].value;
302: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP});
303:
304: (success, result) = callback(element);
305: results[i] = abi.encode(element, success, result);
306: }Using assembly for simple zero checks on unsigned integers can save gas due to lower-level, optimized operations.
Resolution: Implement inline assembly with Solidity's assembly block to perform zero checks. Ensure thorough testing and verification, as assembly lacks the safety checks of high-level Solidity, potentially introducing vulnerabilities if not used carefully.
Num of instances: 6
Click to show findings
['936']
936: if (numOfControllers == 0) return (true, ""); // <= FOUND['115']
115: if (numElements == 0) return false; // <= FOUND['209']
209: if (numElements == 0) return output; // <= FOUND['256']
256: if (numElements == 0) return; // <= FOUND['290']
290: if (numElements == 0) return results; // <= FOUND['1222']
1222: return operatorLookup[addressPrefix][operator] & bitMask != 0; // <= FOUNDUsing nested if statements instead of logical AND (&&) operators can potentially save gas in Solidity contracts. When a series of conditions are connected with &&, all conditions must be evaluated even if the first one fails. In contrast, nested if statements allow for short-circuiting; if the first condition fails, the rest are skipped, saving gas. This approach is more gas-efficient, especially when dealing with complex or gas-intensive conditions. However, it's crucial to balance gas savings with code readability and maintainability, ensuring that the code remains clear and easy to understand.
Num of instances: 17
Click to show findings
['375']
375: if (owner != msgSender && operator != msgSender) { // <= FOUND
376: revert EVC_NotAuthorized();
377: }['499']
499: if (inPermitSelfCall() || (sender != address(0) && sender != msg.sender)) { // <= FOUND
500: revert EVC_NotAuthorized();
501: }['531']
531: if (
532: signer != recoverECDSASigner(permitHash, signature)
533: && !isValidERC1271Signature(signer, permitHash, signature) // <= FOUND
534: ) {
535: revert EVC_NotAuthorized();
536: }['783']
783: if (!authenticated && allowOperator && isAccountOperatorAuthorizedInternal(account, msgSender)) { // <= FOUND
784: authenticated = true;
785: }['793']
793: if (checkLockdownMode && lockdownMode) { // <= FOUND
794: revert EVC_LockdownMode();
795: }['297']
297: if (!enabled && (executionContext.areChecksDeferred() || inPermitSelfCall())) { // <= FOUND
298: revert EVC_NotAuthorized();
299: }['315']
315: if (!enabled && executionContext.areChecksDeferred()) { // <= FOUND
316: revert EVC_NotAuthorized();
317: }['297']
297:
298:
299:
300: if (!enabled && (executionContext.areChecksDeferred() || inPermitSelfCall())) { // <= FOUND['315']
315:
316:
317:
318: if (!enabled && executionContext.areChecksDeferred()) { // <= FOUND['375']
375:
376: if (owner != msgSender && operator != msgSender) { // <= FOUND['499']
499:
500:
501: if (inPermitSelfCall() || (sender != address(0) && sender != msg.sender)) { // <= FOUND['531']
531: if (
532: signer != recoverECDSASigner(permitHash, signature)
533: && !isValidERC1271Signature(signer, permitHash, signature) // <= FOUND
534: ) {['783']
783:
784: if (!authenticated && allowOperator && isAccountOperatorAuthorizedInternal(account, msgSender)) { // <= FOUND['793']
793:
794: if (checkLockdownMode && lockdownMode) { // <= FOUND['943']
943: isValid = success && result.length == 32 // <= FOUND
944: && abi.decode(result, (bytes32)) == bytes32(IVault.checkAccountStatus.selector); // <= FOUND['981']
981: isValid =
982: success && result.length == 32 && abi.decode(result, (bytes32)) == bytes32(IVault.checkVaultStatus.selector); // <= FOUND['1144']
1144: isValid = success && result.length == 32 // <= FOUND
1145: && abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector); // <= FOUNDIn Solidity, caching global or state variables in local variables can be more costly than directly using the state variables. Each assignment to a local variable consumes gas for memory allocation. Accessing state variables directly avoids this overhead, especially if they are only read once, thereby saving gas.
Num of instances: 1
Optimizing low-level calls using assembly in Solidity can be beneficial, particularly when dealing with function return data. Typically, even if return data from a low-level call is not used, Solidity still allocates memory to store it, which incurs gas costs. By using assembly, developers can bypass the automatic memory allocation for unused return data. This manual optimization involves handling the call at the assembly level and deliberately choosing not to store the return data in memory when it's not needed.
Num of instances: 6
Click to show findings
['862']
862: (success, result) = targetContract.call{value: value}(data); // <= FOUND['940']
940: (success, result) =
941: controller.call(abi.encodeCall(IVault.checkAccountStatus, (account, accountCollaterals[account].get()))); // <= FOUND['979']
979: (success, result) = vault.call(abi.encodeCall(IVault.checkVaultStatus, ())); // <= FOUND['862']
862: (success, result) = targetContract.call{value: value}(data); // <= FOUND['940']
940: (success, result) =
941: controller.call(abi.encodeCall(IVault.checkAccountStatus, (account, accountCollaterals[account].get()))); // <= FOUND['979']
979: (success, result) = vault.call(abi.encodeCall(IVault.checkVaultStatus, ())); // <= FOUNDUnused modifiers in a Solidity contract can clutter the codebase and lead to confusion. It's best to remove any modifiers that are not actively used in the contract. This cleanup enhances code readability and maintainability, making it easier for developers to understand and audit the contract's functionality. Additionally, removing redundant code elements can slightly reduce deployment costs.
Num of instances: 2
Click to show findings
['36']
36: modifier callThroughEVC() virtual { // <= FOUND
37: if (msg.sender == address(evc)) {
38: _;
39: } else {
40: address _evc = address(evc);
41:
42: assembly {
43: mstore(0, 0x1f8b521500000000000000000000000000000000000000000000000000000000)
44: mstore(4, address())
45: mstore(36, caller())
46: mstore(68, callvalue())
47: mstore(100, 128)
48: mstore(132, calldatasize())
49: calldatacopy(164, 0, calldatasize())
50:
51:
52:
53: mstore(add(164, calldatasize()), 0)
54: let result := call(gas(), _evc, callvalue(), 0, add(164, and(add(calldatasize(), 31), not(31))), 0, 0)
55:
56: returndatacopy(0, 0, returndatasize())
57: switch result
58: case 0 { revert(0, returndatasize()) }
59: default { return(64, sub(returndatasize(), 64)) }
60: }
61: }
62: }['66']
66: modifier onlyEVCWithChecksInProgress() virtual { // <= FOUND
67: if (msg.sender != address(evc) || !evc.areChecksInProgress()) {
68: revert NotAuthorized();
69: }
70:
71: _;
72: }Please elaborate and generalise the following with detail and feel free to use your own knowledge and lmit ur words to 100 words please:
Num of instances: 4
Click to show findings
['265']
265: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) { // <= FOUND
266: address element = setStorage.elements[i].value; // <= FOUND
267: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP});
268:
269: callback(element);
270: }['300']
300: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) { // <= FOUND
301: address element = setStorage.elements[i].value; // <= FOUND
302: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP});
303:
304: (success, result) = callback(element);
305: results[i] = abi.encode(element, success, result);
306: }['265']
265: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) { // <= FOUND
266: address element = setStorage.elements[i].value; // <= FOUND
267: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP});
268:
269: callback(element);
270: }['300']
300: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) { // <= FOUND
301: address element = setStorage.elements[i].value; // <= FOUND
302: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP});
303:
304: (success, result) = callback(element);
305: results[i] = abi.encode(element, success, result);
306: }If a internal function is only used once it doesn't make sense to modularise it unless the function which does call the function would be overly long and complex otherwise
Num of instances: 15
Click to show findings
['1041']
1041: function isSignerValid(address signer) internal pure virtual returns (bool) // <= FOUND['1056']
1056: function getPermitHash( // <= FOUND
1057: address signer,
1058: address sender,
1059: uint256 nonceNamespace,
1060: uint256 nonce,
1061: uint256 deadline,
1062: uint256 value,
1063: bytes calldata data
1064: ) internal view returns (bytes32 permitHash) ['1091']
1091: function recoverECDSASigner(bytes32 hash, bytes memory signature) internal pure returns (address signer) // <= FOUND['1134']
1134: function isValidERC1271Signature( // <= FOUND
1135: address signer,
1136: bytes32 hash,
1137: bytes memory signature
1138: ) internal view returns (bool isValid) ['67']
67: function setControlCollateralInProgress(EC self) internal pure returns (EC result) // <= FOUND['240']
240: function isOperatorAuthenticated() external view returns (bool) // <= FOUND['71']
71: function isOperatorAuthenticated(EC self) internal pure returns (bool result) // <= FOUND['75']
75: function setOperatorAuthenticated(EC self) internal pure returns (EC result) // <= FOUND['79']
79: function clearOperatorAuthenticated(EC self) internal pure returns (EC result) // <= FOUND['245']
245: function isSimulationInProgress() external view returns (bool) // <= FOUND['83']
83: function isSimulationInProgress(EC self) internal pure returns (bool result) // <= FOUND['87']
87: function setSimulationInProgress(EC self) internal pure returns (EC result) // <= FOUND['442']
442: function reorderCollaterals( // <= FOUND
443: address account,
444: uint8 index1,
445: uint8 index2
446: ) public payable virtual nonReentrantChecksAndControlCollateral onlyOwnerOrOperator(account) ['176']
176: function reorder(SetStorage storage setStorage, uint8 index1, uint8 index2) internal // <= FOUND['223']
223: function getMetadata(SetStorage storage setStorage) internal view returns (uint80) // <= FOUNDNum of instances: 3
Click to show findings
['20']
20: constructor(address _evc) {
21: if (_evc == address(0)) revert EVC_InvalidAddress();
22:
23: evc = IEVC(_evc);
24: }['94']
94: constructor() {
95: CACHED_CHAIN_ID = block.chainid;
96: CACHED_DOMAIN_SEPARATOR = calculateDomainSeparator();
97: }['27']
27: constructor() {
28:
29:
30: executionContext = EC.wrap(1 << ExecutionContext.STAMP_OFFSET);
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48: accountStatusChecks.initialize();
49: vaultStatusChecks.initialize();
50: }Rather defining the struct in a single line, it is more efficient to declare an empty struct and then assign each struct element individually. This can net quite a large gas saving of 130 per instance.
Num of instances: 5
Click to show findings
['344']
344: function setOperator(bytes19 addressPrefix, address operator, uint256 operatorBitField) public payable virtual { // <= FOUND
345: address msgSender = // <= FOUND
346: authenticateCaller({addressPrefix: addressPrefix, allowOperator: false, checkLockdownMode: false});
347:
348:
349: if (operator == address(this) || haveCommonOwnerInternal(msgSender, operator)) {
350: revert EVC_InvalidAddress();
351: }
352:
353: if (operatorLookup[addressPrefix][operator] == operatorBitField) {
354: revert EVC_InvalidOperatorStatus();
355: } else {
356: operatorLookup[addressPrefix][operator] = operatorBitField;
357:
358: emit OperatorStatus(addressPrefix, operator, operatorBitField);
359: }
360: }['365']
365: function setAccountOperator(address account, address operator, bool authorized) public payable virtual { // <= FOUND
366: address msgSender = authenticateCaller({account: account, allowOperator: true, checkLockdownMode: false}); // <= FOUND
367:
368:
369:
370:
371:
372: address owner = haveCommonOwnerInternal(account, msgSender) ? msgSender : getAccountOwnerInternal(account);
373:
374:
375: if (owner != msgSender && operator != msgSender) {
376: revert EVC_NotAuthorized();
377: }
378:
379:
380: if (operator == address(this) || haveCommonOwnerInternal(owner, operator)) {
381: revert EVC_InvalidAddress();
382: }
383:
384: bytes19 addressPrefix = getAddressPrefixInternal(account);
385:
386:
387:
388:
389: uint256 bitMask = 1 << (uint160(owner) ^ uint160(account));
390:
391:
392:
393: uint256 oldOperatorBitField = operatorLookup[addressPrefix][operator];
394: uint256 newOperatorBitField = authorized ? oldOperatorBitField | bitMask : oldOperatorBitField & ~bitMask;
395:
396: if (oldOperatorBitField == newOperatorBitField) {
397: revert EVC_InvalidOperatorStatus();
398: } else {
399: operatorLookup[addressPrefix][operator] = newOperatorBitField;
400:
401: emit OperatorStatus(addressPrefix, operator, newOperatorBitField);
402: }
403: }['1014']
1014: function checkStatusAllWithResult(SetType setType)
1015: internal
1016: virtual
1017: returns (StatusCheckResult[] memory checksResult)
1018: {
1019: bytes[] memory callbackResult = setType == SetType.Account
1020: ? accountStatusChecks.forEachAndClearWithResult(checkAccountStatusInternal)
1021: : vaultStatusChecks.forEachAndClearWithResult(checkVaultStatusInternal);
1022:
1023: uint256 length = callbackResult.length;
1024: checksResult = new StatusCheckResult[](length);
1025:
1026: for (uint256 i; i < length; ++i) {
1027: (address checkedAddress, bool isValid, bytes memory result) =
1028: abi.decode(callbackResult[i], (address, bool, bytes));
1029: checksResult[i] = StatusCheckResult({checkedAddress: checkedAddress, isValid: isValid, result: result}); // <= FOUND
1030: }
1031: }['251']
251: function forEachAndClear(SetStorage storage setStorage, function(address) callback) internal { // <= FOUND
252: uint256 numElements = setStorage.numElements;
253: address firstElement = setStorage.firstElement;
254: uint80 metadata = setStorage.metadata;
255:
256: if (numElements == 0) return;
257:
258: setStorage.numElements = 0;
259: setStorage.firstElement = address(0);
260: setStorage.metadata = metadata;
261: setStorage.stamp = DUMMY_STAMP;
262:
263: callback(firstElement);
264:
265: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
266: address element = setStorage.elements[i].value;
267: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP}); // <= FOUND
268:
269: callback(element);
270: }
271: }['281']
281: function forEachAndClearWithResult(
282: SetStorage storage setStorage,
283: function(address) returns (bool, bytes memory) callback
284: ) internal returns (bytes[] memory) {
285: uint256 numElements = setStorage.numElements;
286: address firstElement = setStorage.firstElement;
287: uint80 metadata = setStorage.metadata;
288: bytes[] memory results = new bytes[](numElements);
289:
290: if (numElements == 0) return results;
291:
292: setStorage.numElements = 0;
293: setStorage.firstElement = address(0);
294: setStorage.metadata = metadata;
295: setStorage.stamp = DUMMY_STAMP;
296:
297: (bool success, bytes memory result) = callback(firstElement);
298: results[0] = abi.encode(firstElement, success, result);
299:
300: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
301: address element = setStorage.elements[i].value;
302: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP}); // <= FOUND
303:
304: (success, result) = callback(element);
305: results[i] = abi.encode(element, success, result);
306: }
307:
308: return results;
309: }Internal functions which are never used use unnecessary gas and should be safely removed.
Num of instances: 2
Click to show findings
['93']
93: function _msgSenderForBorrow() internal view virtual returns (address) // <= FOUND['196']
196: function setMetadata(SetStorage storage setStorage, uint80 metadata) internal // <= FOUNDEmitting variable literals (true, false, 'hello', 1 etc...) in events is inefficient, as it consumes extra gas without providing added value. These literals are fixed values that can be accessed or hardcoded elsewhere in the smart contract or application, making their inclusion in events redundant and an unnecessary drain on resources during transaction execution.
Num of instances: 4
Click to show findings
['436']
436: emit CollateralStatus(account, vault, false); // <= FOUND['479']
479: emit ControllerStatus(account, msg.sender, false); // <= FOUND['425']
425: emit CollateralStatus(account, vault, true); // <= FOUND['471']
471: emit ControllerStatus(account, vault, true); // <= FOUNDShort-circuit evaluation in programming optimizes conditional statements by evaluating expressions in order of least computational complexity. Applying this to smart contracts, especially those on blockchain networks where computation costs gas, can significantly reduce unnecessary operations and associated costs. Prioritizing checks against constants, literals, and local state over function calls ensures that if a condition can be resolved without interacting with function calls, it is.
Num of instances: 2
Click to show findings
['499']
499:
500:
501: if (inPermitSelfCall() || (sender != address(0) && sender != msg.sender)) { // <= FOUND['849']
849:
850:
851:
852:
853:
854:
855:
856: if ( // <= FOUND
857: haveCommonOwnerInternal(onBehalfOfAccount, msgSender) || targetContract == msg.sender // <= FOUND
858: || targetContract == address(this) || contextCache.isControlCollateralInProgress() // <= FOUND
859: ) {The OpenZeppelin Array.unsafeAccess() method is a optimization strategy for Solidity, aimed at reducing gas costs by bypassing automatic length checks on storage array accesses. In Solidity, every access to an array element involves a hidden gas cost due to a length check, ensuring that accesses do not exceed the array bounds. However, if a developer has already verified the array's bounds earlier in the function or knows through logic that the access is safe, directly accessing the array elements without redundant length checks can save gas. This approach requires careful consideration to avoid out-of-bounds errors, as it trades off safety checks for efficiency.
Num of instances: 11
Click to show findings
['607']
607: BatchItem calldata item = items[i]; // <= FOUND['638']
638: (batchItemsResult[i].success, batchItemsResult[i].result) = // <= FOUND
639: callWithAuthenticationInternal(item.targetContract, item.onBehalfOfAccount, item.value, item.data);['1027']
1027: (address checkedAddress, bool isValid, bytes memory result) =
1028: abi.decode(callbackResult[i], (address, bool, bytes)); // <= FOUND['1029']
1029: checksResult[i] = StatusCheckResult({checkedAddress: checkedAddress, isValid: isValid, result: result}); // <= FOUND['61']
61: setStorage.elements[i].stamp = DUMMY_STAMP; // <= FOUND['90']
90: if (setStorage.elements[i].value == element) return false; // <= FOUND['214']
214: output[i] = setStorage.elements[i].value; // <= FOUND['240']
240: if (setStorage.elements[i].value == element) return true; // <= FOUND['266']
266: address element = setStorage.elements[i].value; // <= FOUND['267']
267: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP}); // <= FOUND['305']
305: results[i] = abi.encode(element, success, result); // <= FOUNDTo optimize gas consumption in smart contracts, especially when dealing with loops that write to state variables, a recommended approach is to use a local (memory) variable within the loop for calculations and then update the state variable only once after the loop completes. This method reduces the number of state changes, each of which costs gas, thereby making the operation more gas-efficient.
Num of instances: 2
Click to show findings
['300']
300: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
301: address element = setStorage.elements[i].value;
302: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP});
303:
304: (success, result) = callback(element); // <= FOUND
305: results[i] = abi.encode(element, success, result);
306: }['300']
300: for (uint256 i = EMPTY_ELEMENT_OFFSET; i < numElements; ++i) {
301: address element = setStorage.elements[i].value;
302: setStorage.elements[i] = ElementStorage({value: address(0), stamp: DUMMY_STAMP});
303:
304: (success, result) = callback(element); // <= FOUND
305: results[i] = abi.encode(element, success, result);
306: }In Solidity, the use of uint256 values instead of boolean for certain state variables can result in gas savings. This is due to how Ethereum's storage optimization works: changing a variable from 0 to a non-zero value (like flipping false to true) incurs a higher gas cost compared to modifying an already non-zero value. By using uint256 with values 1 and 2 instead of true and false, you avoid the higher cost associated with the 0 to non-zero change, since 1 and 2 are both non-zero. This approach is notably used in OpenZeppelin's ReentrancyGuard as a gas optimization technique. However, this should be applied where it makes sense and where gas optimization is critical, as it can decrease code readability.
Num of instances: 3
Click to show findings
['776']
776: authenticated = true; // <= FOUND['776']
776: authenticated = true; // <= FOUND['768']
768: bool authenticated = false; // <= FOUNDNum of instances: 1
Consider saving the address(this) value within a constant using foundry's script.sol or solady's LibRlp.sol to save gas
Num of instances: 14
Click to show findings
['98']
98: (sender, controllerEnabled) = evc.getCurrentOnBehalfOfAccount(address(this)); // <= FOUND['100']
100: controllerEnabled = evc.isControllerEnabled(sender, address(this)); // <= FOUND['349']
349:
350: if (operator == address(this) || haveCommonOwnerInternal(msgSender, operator)) { // <= FOUND['380']
380:
381: if (operator == address(this) || haveCommonOwnerInternal(owner, operator)) { // <= FOUND['422']
422: if (vault == address(this)) revert EVC_InvalidAddress(); // <= FOUND['546']
546:
547:
548: (bool success, bytes memory result) = callWithContextInternal(address(this), signer, value, data); // <= FOUND['663']
663: (bool success, bytes memory result) = address(this).delegatecall(abi.encodeCall(this.batchRevert, items)); // <= FOUND['834']
834: value = address(this).balance; // <= FOUND['835']
835: } else if (value > address(this).balance) { // <= FOUND['849']
849:
850:
851:
852:
853:
854:
855:
856: if (
857: haveCommonOwnerInternal(onBehalfOfAccount, msgSender) || targetContract == msg.sender
858: || targetContract == address(this) || contextCache.isControlCollateralInProgress() // <= FOUND
859: ) {['883']
883: if (targetContract == address(this)) { // <= FOUND['893']
893:
894: (success, result) = address(this).delegatecall(data); // <= FOUND['1151']
1151: return keccak256(abi.encode(TYPE_HASH, HASHED_NAME, HASHED_VERSION, block.chainid, address(this))); // <= FOUND['1168']
1168: return address(this) == msg.sender; // <= FOUNDIn smart contract development, utilizing constants for known maximum or minimum values, rather than computing type(uint<n>).max or assuming 0 for .min, can significantly reduce gas costs. Constants require less runtime computation and storage, optimizing contract efficiency—a crucial strategy for developers aiming for cost-effective and performant code.
Num of instances: 2