diff --git a/.github/ISSUE_TEMPLATE/security-check-proposal--scp-.md b/.github/ISSUE_TEMPLATE/security-check-proposal--scp-.md index 1cf431b..e139c8c 100644 --- a/.github/ISSUE_TEMPLATE/security-check-proposal--scp-.md +++ b/.github/ISSUE_TEMPLATE/security-check-proposal--scp-.md @@ -7,9 +7,9 @@ assignees: '' --- -ATTENTION! If you would like to submit a SCP please use the following schema (described in [template](https://github.com/securing/SCSVS/blob/prerelease/SCSVSv2/scp-template.md)). +ATTENTION! If you would like to submit a SCP please use the following schema (described in [template](https://github.com/ComposableSecurity/SCSVS/blob/prerelease/SCSVSv2/scp-template.md)). -If you have a ready change to be added to the repository, please submit it as a [Pull Request](https://github.com/securing/SCSVS/pulls) and create a SCP with link to PR. +If you have a ready change to be added to the repository, please submit it as a [Pull Request](https://github.com/ComposableSecurity/SCSVS/pulls) and create a SCP with link to PR. ## Check diff --git a/1.1/README.md b/1.1/README.md index 637ed6c..fddf3fb 100644 --- a/1.1/README.md +++ b/1.1/README.md @@ -2,7 +2,7 @@ ## Authors -Damian Rusinek (damian.rusinek@securing.pl), Paweł Kuryłowicz (pawel.kurylowicz@securing.pl) +Damian Rusinek (damian.rusinek@composable-security.com), Paweł Kuryłowicz (pawel.kurylowicz@composable-security.com) ## Introduction @@ -27,8 +27,6 @@ You can use the SCSVS checklist in multiple ways: The entire checklist is in a form similar to OWASP APPLICATION SECURITY VERIFICATION STANDARD v4.0. Every category has a brief description of the control objectives and a list of security verification requirements. -[___Download SCSVS PDF version___](SCSVS_v1.1.pdf) - **Key areas that have been included:** * [V1: Architecture, Design and Threat Modelling](./0x10-V1-Architecture-Design-Threat-modelling.md) * [V2: Access Control](./0x11-V2-Access-Control.md) diff --git a/1.2/README.md b/1.2/README.md index 637ed6c..fddf3fb 100644 --- a/1.2/README.md +++ b/1.2/README.md @@ -2,7 +2,7 @@ ## Authors -Damian Rusinek (damian.rusinek@securing.pl), Paweł Kuryłowicz (pawel.kurylowicz@securing.pl) +Damian Rusinek (damian.rusinek@composable-security.com), Paweł Kuryłowicz (pawel.kurylowicz@composable-security.com) ## Introduction @@ -27,8 +27,6 @@ You can use the SCSVS checklist in multiple ways: The entire checklist is in a form similar to OWASP APPLICATION SECURITY VERIFICATION STANDARD v4.0. Every category has a brief description of the control objectives and a list of security verification requirements. -[___Download SCSVS PDF version___](SCSVS_v1.1.pdf) - **Key areas that have been included:** * [V1: Architecture, Design and Threat Modelling](./0x10-V1-Architecture-Design-Threat-modelling.md) * [V2: Access Control](./0x11-V2-Access-Control.md) diff --git a/2.0/0x100-General/0x102-G2-Policies-procedures.md b/2.0/0x100-General/0x102-G2-Policies-procedures.md index 0bdc876..91796ed 100644 --- a/2.0/0x100-General/0x102-G2-Policies-procedures.md +++ b/2.0/0x100-General/0x102-G2-Policies-procedures.md @@ -8,7 +8,7 @@ Think about possible situations and be prepared in case they arise. Act consciou Ensure that your project satisfies the following high-level requirements: * Security procedures and policies are thought out and ready to use, * Procedures and policies cover known and common threats from the past of other DeFi projects, -* Employees are familiar with the policies and procedures and they know what they are responsible for. +* Employees are familiar with the policies and procedures, and they know what they are responsible for. Category “G2” lists requirements related to the policies, and procedures in the context of security of DeFi projects. @@ -24,12 +24,15 @@ Category “G2” lists requirements related to the policies, and procedures in | **G2.6** | Verify that the process of major system changes involves threat modeling by an external company. | | **G2.7** | Verify that the process of adding and updating components to the system includes a security audit by an external company. | | **G2.8** | Verify that there is a clear and known procedure in place in the event of a hack. | -| **G2.9** | Verify that the procedure in the event of hack have defined exact persons to execute required actions. | +| **G2.9** | Verify that the procedure in the event of hack have defined individuals to execute required actions. | | **G2.10** | Verify that the procedure includes alarming other projects about the hack through trusted channels. | | **G2.11** | Verify that a procedure is defined in case one of the project's private keys is leaked. | +| **G2.12** | Verify that the project has an emergency contact with the external company that conducted the last audit. | ## References For more information, see also: -* [TODO](https://www.youtube.com/watch?v=IwR4PAmRhhg&list=PL-lO2xrptAtav4SZgCdDkVxChWhVU3kmP&index=18) +* [Emergency Procedures - Yearn Finance](https://docs.yearn.finance/vaults/0.4.2/process-and-procedures/emergency) +* [Emergency tools - Yearn Finance](https://docs.yearn.finance/vaults/0.4.2/process-and-procedures/emergency#tools) +* [Emergency checklist - Yearn Finance](https://docs.yearn.finance/vaults/0.4.2/process-and-procedures/emergency#emergency-checklist) diff --git a/2.0/0x100-General/0x103-G3-Upgradeability.md b/2.0/0x100-General/0x103-G3-Upgradeability.md index 2730b1f..a2e62fc 100644 --- a/2.0/0x100-General/0x103-G3-Upgradeability.md +++ b/2.0/0x100-General/0x103-G3-Upgradeability.md @@ -33,5 +33,7 @@ Category “G3” lists requirements related to the upgradeabilitiy of the smart For more information, see also: +* [OpenZeppelin upgrades plugins](https://docs.openzeppelin.com/upgrades-plugins) +* [Chainlink upgradable smart contracts](https://blog.chain.link/upgradable-smart-contracts/) * [Contract upgrade anti-patterns](https://blog.trailofbits.com/2018/09/05/contract-upgrade-anti-patterns) -* [Security in Upgrades of Smart Contracts](https://www.youtube.com/watch?v=5WE6PEc305w) \ No newline at end of file +* [Security in Upgrades of Smart Contracts](https://www.youtube.com/watch?v=5WE6PEc305w) diff --git a/2.0/0x100-General/0x104-G4-Business-Logic.md b/2.0/0x100-General/0x104-G4-Business-Logic.md index a5f18a7..6919ebc 100644 --- a/2.0/0x100-General/0x104-G4-Business-Logic.md +++ b/2.0/0x100-General/0x104-G4-Business-Logic.md @@ -2,7 +2,7 @@ ## Control Objective -To build secure smart contracts, we need to consider security of their business logic. Some of the smart contracts are influenced by vulnerabilities by their design. +To build secure smart contracts, we need to consider the security of their business logic. Some of the smart contracts are influenced by vulnerabilities in their design. The components used should not be considered safe without verification. Business assumptions should meet with the principle of minimal trust, which is essential in building smart contracts. Ensure that a verified contract satisfies the following high-level requirements: @@ -18,13 +18,18 @@ Category “G4” lists requirements related to the business logic of the smart | --- | --- | | **G4.1** | Verify that there are no vulnerabilities associated with business logic. | | **G4.2** | Verify that the contract logic and protocol parameters implementation corresponds to the documentation. | -| **G4.3** | Verify that the business logic flows of smart contract proceed in a sequential step order and it is not possible to skip any part of it or to do it in a different order than designed. | -| **G4.4** | Verify that the contract has business limits and correctly enforces it. | +| **G4.3** | Verify that the business logic flows of smart contract proceed in a sequential step order, and it is not possible to skip any part of it or to do it in a different order than designed. | +| **G4.4** | Verify that the contract has business limits and correctly enforces them. | | **G4.5** | Verify that the business logic of contract does not rely on the values retrieved from untrusted contracts (especially when there are multiple calls to the same contract in one flow). | | **G4.6** | Verify that the contract logic does not rely on the balance of contract (e.g., *balance == 0*). | | **G4.7** | Verify that the sensitive operations of contract do not depend on the block data (e.g., *block hash*, *timestamp*). | | **G4.8** | Verify that the contract uses mechanisms that mitigate transaction-ordering dependence (front-running) attacks (e.g. pre-commit scheme). | -| **G4.9** | Verify that the contract does not send funds automatically, but it lets users withdraw funds on their own in separate transaction instead. | +| **G4.9** | Verify that the contract does not send funds automatically, but it lets users withdraw funds on their own in the separate transaction instead. | +| **G4.10** | Verify that the functions are built symmetrically (e.g. in the case of a *deposit*, there is also a *withdraw*). | +| **G4.11** | Verify that calling a function once with value XY gives a similar result as calling it X times with value Y. | +| **G4.12** | Verify that the global state is also updated when working on a copy of some data for optimization reasons. | +| **G4.13** | Verify that when accepting ETH deposits directly and indirectly via WETH, you are validating the case of msg.value > 0 and the amount of WETH transferred in the same function call. | +| **G4.14** | Verify that there are no off-by one errors, especially with *>=*, *<=* and *length - 1*. | ## References @@ -36,3 +41,4 @@ For more information, see also: * [Solidity Recommendations](https://consensys.github.io/smart-contract-best-practices/recommendations/) * [SWC-125 Incorrect Inheritance Order](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-125) * [EIP 1052: EXTCODEHASH Opcode](https://eips.ethereum.org/EIPS/eip-1052) +* [Smart Contract Auditing Heuristics](https://github.com/OpenCoreCH/smart-contract-auditing-heuristics?utm_source=substack&utm_medium=email#readme) diff --git a/2.0/0x100-General/0x105-G5-Access-Control.md b/2.0/0x100-General/0x105-G5-Access-Control.md index a82c328..f26575f 100644 --- a/2.0/0x100-General/0x105-G5-Access-Control.md +++ b/2.0/0x100-General/0x105-G5-Access-Control.md @@ -6,7 +6,7 @@ Access control is the process of granting or denying specific requests to obtain Ensure that a verified contract satisfies the following high-level requirements: * Users and other contracts are associated with a well-defined set of roles and privileges, -* Access is granted only to the privileged users and contracts. +* Access is granted only to privileged users and contracts. Category “G5” lists requirements related to the access control mechanisms of the smart contracts. @@ -16,18 +16,19 @@ Category “G5” lists requirements related to the access control mechanisms of | --- | --- | | **G5.1** | Verify that there are no vulnerabilities associated with access control. | | **G5.2** | Verify that the principle of the least privilege exists. Other contracts should only be able to access functions and data for which they possess specific authorization. | -| **G5.3** | Verify that new contracts with access to the audited contract adhere to the principle of minimum rights by default. Contracts should have a minimal or no permission until access to the new features is explicitly granted. | -| **G5.4** | Verify that the creator of the contract complies with the rule of the least privilege and their rights strictly follow the documentation. | +| **G5.3** | Verify that new contracts with access to the audited contract adhere to the principle of minimum rights by default. Contracts should have minimal or no permission until access to the new features is explicitly granted. | +| **G5.4** | Verify that the creator of the contract complies with the rule of the least privilege and that their rights strictly follow the documentation. | | **G5.5** | Verify that the contract enforces the access control rules specified in a trusted contract, especially if the dApp client-side access control is present and could be bypassed. | | **G5.6** | Verify that the calls to external contracts are allowed only if necessary. | | **G5.7** | Verify that the code of modifiers is clear and simple. The logic should not contain external calls to untrusted contracts. | -| **G5.8** | Verify that all user and data attributes used by access controls are kept in trusted contract and cannot be manipulated by other contracts unless specifically authorized. | +| **G5.8** | Verify that all user and data attributes used by access controls are kept in a trusted contract and cannot be manipulated by other contracts unless specifically authorized. | | **G5.9** | Verify that the access controls fail securely, including when a revert occurs. | | **G5.10** | Verify that if the input (function parameters) is validated, the positive validation approach (allowlisting) is used where possible. | +| **G5.11** | Verify that passing priviledged access to another address is two-step operation. | ## References For more information, see also: * [OWASP Cheat Sheet: Access Control](https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Access_Control_Cheat_Sheet.md) -* [OpenZeppelin: Role-Based Access Control](https://github.com/OpenZeppelin/openzeppelin-solidity/blob/852e11c2dbb19a4000decacf1840f5e4c29c5543/docs/access-control.md#role-based-access-control) +* [OpenZeppelin: Access Control]([https://github.com/OpenZeppelin/openzeppelin-solidity/blob/852e11c2dbb19a4000decacf1840f5e4c29c5543/docs/access-control.md#role-based-access-control](https://docs.openzeppelin.com/contracts/3.x/access-control)) diff --git a/2.0/0x100-General/0x106-G6-Communications.md b/2.0/0x100-General/0x106-G6-Communications.md index 81ddd5d..324a825 100644 --- a/2.0/0x100-General/0x106-G6-Communications.md +++ b/2.0/0x100-General/0x106-G6-Communications.md @@ -2,10 +2,10 @@ ## Control Objective -Communications includes the topic of the relations between smart contracts and their libraries. +Communications include the topic of the relations between smart contracts and their libraries. Ensure that a verified contract satisfies the following high-level requirements: -* The external calls from and to other contracts have considered abuse case and are authorized, +* The external calls *from* and *to* other contracts have considered abuse cases and are authorized, * Used libraries are safe and the state-of-the-art security libraries are used. Category “G6” lists requirements related to the function calls between the verified contracts and other contracts out of the scope of the application. @@ -15,13 +15,14 @@ Category “G6” lists requirements related to the function calls between the v | # | Description | | --- | --- | | **G6.1** | Verify that there are no vulnerabilities associated with communications. | -| **G6.2** | Verify that libraries which are not part of the application (but the smart contract relies on to operate) are identified. | -| **G6.3** | Verify that delegatecall is not used with untrusted contracts. | -| **G6.4** | Verify that third party contracts do not shadow special functions (e.g. revert). | +| **G6.2** | Verify that libraries that are not part of the application (but the smart contract relies on to operate) are identified. | +| **G6.3** | Verify that *delegatecall* is not used with untrusted contracts. | +| **G6.4** | Verify that third-party contracts do not shadow special functions (e.g. *revert*). | | **G6.5** | Verify that the contract does not, check whether the address is a contract using *extcodesize* opcode. | -| **G6.6** | Verify that re-entrancy attack is mitigated by blocking recursive calls from other contracts and following Check-Effects-Interactions pattern. Do not use *send* function unless it is a must. | +| **G6.6** | Verify that re-entrancy attack is mitigated by blocking recursive calls from other contracts and following **Check-Effects-Interactions** pattern. Do not use *send* function unless it is a must. | | **G6.7** | Verify that the result of low-level function calls (e.g. *send*, *delegatecall*, *call*) from another contracts is checked. | -| **G6.8** | Verify that contract relies on the data provided by right sender and contract does not rely on tx.origin value. | +| **G6.8** | Verify that contract relies on the data provided by the right sender and the contract does not rely on *tx.origin* value. | +| **G6.9** | Verify that contract does not enforce usage of "phantom functions". | ## References @@ -31,4 +32,5 @@ For more information, see also: * [DASP 10: Unchecked Return Values For Low Level Calls](https://www.dasp.co/#item-4) * [SWC-107 Reentrancy](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-107) * [SWC-112 Delegatecall to Untrusted Callee](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-112) +* [Phantom functions](https://media.dedaub.com/phantom-functions-and-the-billion-dollar-no-op-c56f062ae49f) diff --git a/2.0/0x100-General/0x107-G7-Arithmetic.md b/2.0/0x100-General/0x107-G7-Arithmetic.md index 1457b35..19e2291 100644 --- a/2.0/0x100-General/0x107-G7-Arithmetic.md +++ b/2.0/0x100-General/0x107-G7-Arithmetic.md @@ -3,7 +3,7 @@ ## Control Objective Ensure that a verified contract satisfies the following high-level requirements: -* All mathematical operations and extreme variable values are handled in a safe and predictable manner. +* All mathematical operations and extreme variable values are handled safely and predictably. Category “G7” lists requirements related to the arithmetic operations of the smart contracts. @@ -12,18 +12,21 @@ Category “G7” lists requirements related to the arithmetic operations of the | # | Description | | --- | --- | | **G7.1** | Verify that there are no vulnerabilities associated with arithmetic. | -| **G7.2** | Verify that the values and math operations are resistant to integer overflows. Use SafeMath library for arithmetic operations before solidity 0.8.*. | -| **G7.3** | Verify that the unchecked code snippets from Solidity 0.8.* do not introduce integer under/overflows. | -| **G7.4** | Verify that the extreme values (e.g. maximum and minimum values of the variable type) are considered and does change the logic flow of the contract. | +| **G7.2** | Verify that the values and math operations are resistant to integer overflows. Use *SafeMath* library for arithmetic operations before solidity 0.8.\*. | +| **G7.3** | Verify that the unchecked code snippets from Solidity 0.8.\* do not introduce integer under/overflows. | +| **G7.4** | Verify that the extreme values (e.g. maximum and minimum values of the variable type) are considered and do change the logic flow of the contract. | | **G7.5** | Verify that non-strict inequality is used for balance equality. | | **G7.6** | Verify that there is a correct order of magnitude in the calculations. | | **G7.7** | Verify that in calculations, multiplication is performed before division for accuracy. | | **G7.8** | Verify that contract does not assume fixed-point precision and use a multiplier or store both the numerator and denominator. | +| **G7.9** | Verify that rounding uses secure granularity (e.g. seconds instead of days). | +| **G7.10** | Verify that rounding in extreme conditions (e.g. very often) does not cause amplified losses (e.g. lack of interests accrued). Use RayWad library for such calculations. | ## References For more information, see also: -* [DASP 10: Artithmetic Issues](https://www.dasp.co/#item-3) +* [DASP 10: Arithmetic Issues](https://www.dasp.co/#item-3) * [OpenZeppelin: SafeMath](https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/contracts/math/SafeMath.sol) * [SWC-101 Integer Overflow and Underflow](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-101) +* [Significant Rounding Errors For Interest Calculation](https://github.com/OpenCoreCH/smart-contract-audits/blob/main/reports/c4/rigor.md#high-significant-rounding-errors-for-interest-calculation) diff --git a/2.0/0x100-General/0x108-G8-Denial-of-Service.md b/2.0/0x100-General/0x108-G8-Denial-of-Service.md index f02ff99..680ca7b 100644 --- a/2.0/0x100-General/0x108-G8-Denial-of-Service.md +++ b/2.0/0x100-General/0x108-G8-Denial-of-Service.md @@ -14,14 +14,14 @@ Category “G8” lists requirements related to the possible denial of service o | **G8.1** | Verify that there are no vulnerabilities associated with Denial of Service. | | **G8.2** | Verify that the contract does not iterate over unbound loops. | | **G8.3** | Verify that self-destruct functionality is used only if necessary. If it is included in the contract, it should be clearly described in the documentation. | -| **G8.4** | Verify that the business logic does not block its flows when any of the participants is absent forever. | -| **G8.5** | Verify that the contract logic does not disincentivize users to use contracts (e.g. the cost of transaction is higher than the profit). | -| **G8.6** | Verify that expressions of functions assert or require have a passing variant. | -| **G8.7** | Verify that if fallback function is not callable by anyone, it is not blocking the functionalities of contract | +| **G8.4** | Verify that the business logic does not block its flows when any of the participants are absent forever. | +| **G8.5** | Verify that the contract logic does not disincentivize users to use contracts (e.g. the cost of the transaction is higher than the profit). | +| **G8.6** | Verify that expressions of functions *assert* or *require* have a passing variant. | +| **G8.7** | Verify that if *fallback* function is not callable by anyone, it is not blocking the functionalities of the contract | | **G8.8** | Verify that there are no costly operations in a loop. | | **G8.9** | Verify that there are no calls to untrusted contracts in a loop. | | **G8.10** | Verify that if there is a possibility of suspending the operation of the contract, it is also possible to resume it. | -| **G8.11** | Verify that if allow lists and list deny are used, it does not interfere with normal operation of the system. | +| **G8.11** | Verify that if allow lists and deny lists are used, it does not interfere with the normal operation of the system. | | **G8.12** | Verify that there is no DoS caused by overflows and underflows. | ## References @@ -34,4 +34,4 @@ For more information, see also: * [DoS with Block Gas Limit](https://consensys.github.io/smart-contract-best-practices/known_attacks/#dos-with-block-gas-limit) * [SWC-128 DoS With Block Gas Limit](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-128) * [SWC-113 DoS with Failed Call](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-113) -* [Uncallable function example](https://github.com/ethereum/EIPs/issues/820#issuecomment-454021564) \ No newline at end of file +* [Uncallable function example](https://github.com/ethereum/EIPs/issues/820#issuecomment-454021564) diff --git a/2.0/0x100-General/0x109-G9-Blockchain-Data.md b/2.0/0x100-General/0x109-G9-Blockchain-Data.md index 01ffbae..e45f9bd 100644 --- a/2.0/0x100-General/0x109-G9-Blockchain-Data.md +++ b/2.0/0x100-General/0x109-G9-Blockchain-Data.md @@ -2,10 +2,10 @@ ## Control Objective -Smart contracts in public blockchains have no built-in mechanism to store secret data securely. It is important to protect sensitive data from reading by an untrusted actor. +Smart contracts in public blockchains have no built-in mechanism to store secret data securely. It is important to protect sensitive data from being read by an untrusted actor. Ensure that a verified contract satisfies the following high-level requirements: -* Data stored in smart contract is identified and protected, +* Data stored in a smart contract is identified and protected, * Secret data is not kept in blockchain as plaintext, * Smart contract is not vulnerable due to data misrepresentation. @@ -19,12 +19,15 @@ Category “G9” lists requirements related to the blockchain data of the smart | **G9.2** | Verify that any data saved in contracts is not considered secure or private (even private variables). | | **G9.3** | Verify that no confidential data is stored in the blockchain (passwords, personal data, token etc.). | | **G9.4** | Verify that contract does not use string literals as keys for mappings. Verify that global constants are used instead to prevent Homoglyph attack. | -| **G9.5** | Verify that contract does not generate pseudorandom numbers trivially basing on the information from blockchain (e.g. seeding with the block number). | +| **G9.5** | Verify that contract does not generate pseudorandom numbers trivially basing on the information from blockchain (e.g. seeding with the block number or block timestamp). | +| **G9.6** | Don't rely on block hash `block.blockhash(uint blockNumber)` for blocks older than 256 from curent block, as they always will have 0 value. | ## References For more information, see also: +* [eth2book Randomness](https://eth2book.info/altair/part2/building_blocks/randomness) +* [Chainlink VRF](https://docs.chain.link/vrf/v2/introduction/) * [Unencrypted Private Data On-Chain](https://swcregistry.io/docs/SWC-136) * [Homoglyph attack](https://github.com/Arachnid/uscc/tree/master/submissions-2017/marcogiglio) diff --git a/2.0/0x100-General/0x110-G10-Gas.md b/2.0/0x100-General/0x110-G10-Gas.md index 267e659..f03232d 100644 --- a/2.0/0x100-General/0x110-G10-Gas.md +++ b/2.0/0x100-General/0x110-G10-Gas.md @@ -2,7 +2,7 @@ ## Control Objective -The efficiency of gas consumption should be taken into account not only in terms of optimization, but also in terms of safety to avoid possible exhaustion. Smart contracts by nature have different limitations that, if they are not taken into account, can cause different vulnerabilities. +The efficiency of gas consumption should be taken into account not only in terms of optimization but also in terms of safety to avoid possible exhaustion. Smart contracts by nature have different limitations that, if they are not taken into account, can cause different vulnerabilities. Ensure that a verified contract satisfies the following high-level requirements: * The use of gas is optimized and unnecessary losses are prevented, @@ -15,8 +15,9 @@ Category “G10” lists requirements related to gas and smart contract limitati | # | Description | | --- | --- | | **G10.1** | Verify that there are no vulnerabilities associated with gas. | -| **G10.2** | Verify that the usage of gas in a smart contract is anticipated, defined and have clear limitations that cannot be exceeded. Both, code structure and malicious input should not cause gas exhaustion. | +| **G10.2** | Verify that the usage of gas in a smart contract is anticipated, defined, and has clear limitations that cannot be exceeded. Both, code structure and malicious input should not cause gas exhaustion. | | **G10.3** | Verify that there is no hard-coded amount of gas assigned to the function call (the gas prices may vary in the future). | +| **G10.4** | Verify that the contract does not allow for inflating gas via "return bomb" attack. | ## References @@ -24,4 +25,5 @@ For more information, see also: * [Gas Limit and Loops](https://solidity.readthedocs.io/en/v0.5.10/security-considerations.html#gas-limit-and-loops) * [Insufficient gas griefing](https://consensys.github.io/smart-contract-best-practices/known_attacks/#insufficient-gas-griefing) -* [EIP 150: Gas cost changes for IO-heavy operations](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-150.md) \ No newline at end of file +* [EIP 150: Gas cost changes for IO-heavy operations](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-150.md) +* [Return bomb attack explained](https://github.com/nomad-xyz/ExcessivelySafeCall) \ No newline at end of file diff --git a/2.0/0x100-General/0x111-G11-Code-Clarity.md b/2.0/0x100-General/0x111-G11-Code-Clarity.md index 76d8a9b..2ececeb 100644 --- a/2.0/0x100-General/0x111-G11-Code-Clarity.md +++ b/2.0/0x100-General/0x111-G11-Code-Clarity.md @@ -14,11 +14,11 @@ Category “G11” lists requirements related to the code clarity of the smart c | # | Description | | --- | --- | -| **G11.1** | Verify that there are no recommendations regarding needed improvement of code clarity in the external company audit report. | +| **G11.1** | Verify that there are no recommendations regarding the needed improvement of code clarity in the external company audit report. | | **G11.2** | Verify that the logic is clear and modularized in multiple simple contracts and functions. | -| **G11.3** | Verify that there is a description in the form of 1-2 short sentences of what the contract is for at the beginning of the contract. | +| **G11.3** | Verify that there is a description in the form of at least 1-2 short sentences of what the contract is for at the beginning of the contract. | | **G11.4** | Verify that if the system uses a ready-made implementation of the contract, it has been marked in the comment. If it contains changes from the original, those have been specified. | -| **G11.5** | Verify that the inheritance order is considered for contracts that use multiple inheritance and shadow functions. | +| **G11.5** | Verify that the inheritance order is considered for contracts that use multiple inheritances and shadow functions. | | **G11.6** | Verify that the contract uses existing and tested code (e.g. token contract or mechanisms like *ownable*) instead of implementing its own. | | **G11.7** | Verify that the same rules for variable naming are followed throughout all the contracts (e.g. use the same variable name for the same object). | | **G11.8** | Verify that variables with similar names are not used. | @@ -33,5 +33,5 @@ Category “G11” lists requirements related to the code clarity of the smart c For more information, see also: -* [Solidity Style Guide v0.8.10](https://docs.soliditylang.org/en/v0.8.10/style-guide.html) -* [Solidity: Security Considerations & Recommendations](https://solidity.readthedocs.io/en/v0.5.10/security-considerations.html#recommendations) \ No newline at end of file +* [Solidity Style Guide v0.8.17](https://docs.soliditylang.org/en/v0.8.17/style-guide.html) +* [Solidity: Security Considerations & Recommendations](https://solidity.readthedocs.io/en/v0.5.10/security-considerations.html#recommendations) diff --git a/2.0/0x100-General/0x112-G12-Test-Coverage.md b/2.0/0x100-General/0x112-G12-Test-Coverage.md index 3130e35..f1c5da5 100644 --- a/2.0/0x100-General/0x112-G12-Test-Coverage.md +++ b/2.0/0x100-General/0x112-G12-Test-Coverage.md @@ -15,20 +15,21 @@ Category “G12” lists requirements related to the testing process of the smar | --- | --- | | **G12.1** | Verify that there are no recommendations regarding the needed improvement test coverage in the external company audit report. | | **G12.2** | Verify that abuser stories specified during threat modeling are covered by unit tests. | -| **G12.3** | Verify that considered as sensitive functions of verified contract are covered with tests in the development phase. | -| **G12.4** | Verify that the implementation of verified contract has been checked for security vulnerabilities using static and dynamic analysis. | -| **G12.5** | Verify that the specification of smart contract has been formally verified. | -| **G12.6** | Verify that the specification and the result of formal verification is included in the documentation. | +| **G12.3** | Verify that sensitive functions of the verified contract are covered with tests in the development phase. | +| **G12.4** | Verify that the implementation of the verified contract has been checked for security vulnerabilities using static and dynamic analysis. | +| **G12.5** | Verify that the specification of the smart contract has been formally verified. | +| **G12.6** | Verify that the specification and the result of formal verification are included in the documentation. | +| **G12.7** | Verify that *solidity-coverage* indicates excellent code coverage. | ## References For more information, see also: -* [Formal Verification](https://solidity.readthedocs.io/en/v0.5.10/security-considerations.html#formal-verification) +* [Formal Verification](https://docs.soliditylang.org/en/latest/smtchecker.html#smtchecker-and-formal-verification) +* [Foundry](https://github.com/foundry-rs/foundry) +* [Slither](https://github.com/crytic/slither) * [Code coverage for Solidity testing](https://github.com/sc-forks/solidity-coverage) -* [Remix IDE](https://remix.ethereum.org/) * [MythX Plugin for Truffle](https://github.com/ConsenSys/truffle-security) * [Securify](https://securify.chainsecurity.com/) * [SmartCheck](https://tool.smartdec.net/) * [Oyente](https://github.com/melonproject/oyente) -* [Security Tools](https://consensys.github.io/smart-contract-best-practices/security_tools/) \ No newline at end of file diff --git a/2.0/0x200-Components/0x201-C1-Token.md b/2.0/0x200-Components/0x201-C1-Token.md index d8da397..3af2c09 100644 --- a/2.0/0x200-Components/0x201-C1-Token.md +++ b/2.0/0x200-Components/0x201-C1-Token.md @@ -14,15 +14,16 @@ Category “C1” lists requirements related to the Token smart contract as one | # | Description | | --- | --- | -| **C1.1** | Verify that there are no vulnerabilities associated with Token component. | -| **C1.2** | Verify that token contract follows tested and stable Token standard. | +| **C1.1** | Verify that there are no vulnerabilities associated with the Token component. | +| **C1.2** | Verify that the token contract follows tested and stable Token standards. | | **C1.3** | Verify that the security considerations of the standard used are known and handled by the team. | | **C1.4** | Verify that the total token supply matches the documentation. | | **C1.5** | Verify that the number of minted and burned tokens does not disturb the operation of the system specified in the documentation. | | **C1.6** | Verify that if the token contains a fee, its maximum value is predetermined and matches the documentation. | -| **C1.7** | Verify that the transfer business logic is consistent, especially when re-sending tokens to the same address (from == to). | +| **C1.7** | Verify that the transfer business logic is consistent, especially when re-sending tokens to the same address (*from == to*). | | **C1.8** | Verify that if the implemented functions include external calls, they are handled correctly and do not introduce unsafe external calls to the system. | -| **C1.9** | Verify that the *approve()* function from the ERC-20 standard change the allowed amount only to 0 or from 0. | +| **C1.9** | Verify that the *approve()* function from the ERC-20 standard change the allowed amount only to *0* or from *0*. | +| **C1.10** | Verify that the token does not have double entry point and does not track user balances in more than one smart contract. | ## References @@ -30,4 +31,4 @@ For more information, see also: * [Token Implementation Best Practice](https://consensys.github.io/smart-contract-best-practices/tokens/) * [iToken Duplication Incident Report](https://bzx.network/blog/incident) -* [The Dangers of Token Integration](https://www.youtube.com/watch?v=6GaCt_lM_ak) \ No newline at end of file +* [The Dangers of Token Integration](https://www.youtube.com/watch?v=6GaCt_lM_ak) diff --git a/2.0/0x200-Components/0x202-C2-Governance.md b/2.0/0x200-Components/0x202-C2-Governance.md index c65ea51..a6aee6e 100644 --- a/2.0/0x200-Components/0x202-C2-Governance.md +++ b/2.0/0x200-Components/0x202-C2-Governance.md @@ -15,17 +15,21 @@ Category “C2” lists requirements related to the Governance smart contract as | # | Description | | --- | --- | -| **C2.1** | Verify that there are no vulnerabilities associated with Governance component. | -| **C2.2** | Verify that Governance contract follows tested and stable Governance standard. | +| **C2.1** | Verify that there are no vulnerabilities associated with the Governance component. | +| **C2.2** | Verify that Governance contract follows tested and stable Governance standards. | | **C2.3** | (DAO) Verify that the governance contract is protected from the attacks that use flash loans. | -| **C2.4** | (DAO) Verify that the votes over taking actions by governance are counted correctly, in accordance with the documentation. | +| **C2.4** | (DAO) Verify that the votes over taking actions by governance are counted correctly, following the documentation. | | **C2.5** | (DAO) Verify that the delays associated with the enforcement of actions are consistent with those described in the documentation. | -| **C2.6** | Verify that special actions that can be performed only by Governance are clearly marked in the documentation. | -| **C2.7** | Verify that the Governance cannot do more than declared in the documentation. | -| **C2.8** | Verify that the ownership transfer is a 2-step process where the candidate accepts it in a separate transaction and the previous owner does not lose privileges until it is done. | +| **C2.6** | (DAO) Verify that the actions can only be performed after voting is completed (unless explicitly stated otherwise in the documentation). +| **C2.7** | (DAO) Verify that the actions can be performed only following the voting result (voted yes - must be performed, voted no or not obtaining the required number of votes - cannot be performed). +| **C2.8** | Verify that special actions that can be performed only by Governance are marked in the documentation. | +| **C2.9** | Verify that the Governance cannot do more than declared in the documentation. | +| **C2.10** | Verify that the ownership transfer is a 2-step process where the candidate accepts it in a separate transaction and the previous owner does not lose privileges until it is done. | +| **C2.11** | Verify that key operations on the Governance contract can only be performed with the appropriate permissions. Functions in this contract should not be available to general users.| ## References For more information, see also: * [Strategies for Secure Governance with Smart Contracts](https://www.youtube.com/watch?v=GbDAmMdmh8Q) +* [MakerDAO Governance Module](https://docs.makerdao.com/smart-contract-modules/governance-module) diff --git a/2.0/0x200-Components/0x203-C3-Oracle.md b/2.0/0x200-Components/0x203-C3-Oracle.md index 5101512..f21a716 100644 --- a/2.0/0x200-Components/0x203-C3-Oracle.md +++ b/2.0/0x200-Components/0x203-C3-Oracle.md @@ -15,11 +15,12 @@ Category “C3” lists requirements related to the Oracle smart contract as one | # | Description | | --- | --- | -| **C3.1** | Verify that there are no vulnerabilities associated with Oracle component. | +| **C3.1** | Verify that there are no vulnerabilities associated with the Oracle component. | | **C3.2** | Verify that the manipulation of the returned data by Oracle is unprofitable for the attacker. | | **C3.3** | Verify that there are alerts set and monitored for large and sudden changes in the price feed. | | **C3.4** | Verify if there is a way to mark the data as incorrect. | | **C3.5** | Verify that the supply with incorrect data is penalized. | +| **C3.6** | Verify that the value (e.g., price for an asset) returned by oracle cannot be influenced in a single block. | ## References @@ -27,4 +28,4 @@ For more information, see also: * [ORACLES](https://ethereum.org/en/developers/docs/oracles/) * [The Dangers of Price Oracles in Smart Contracts](https://www.youtube.com/watch?v=YGO7nzpXCeA) -* [So you want to use a price oracle](https://samczsun.com/so-you-want-to-use-a-price-oracle/) \ No newline at end of file +* [So you want to use a price oracle](https://samczsun.com/so-you-want-to-use-a-price-oracle/) diff --git a/2.0/0x200-Components/0x204-C4-Vault.md b/2.0/0x200-Components/0x204-C4-Vault.md index e69de29..5d59c22 100644 --- a/2.0/0x200-Components/0x204-C4-Vault.md +++ b/2.0/0x200-Components/0x204-C4-Vault.md @@ -0,0 +1,41 @@ +# C4: Vault + +## Control Objective + +If a project builds a Vault smart contract, it is necessary to follow the standard and create a secure contract based on it. Learn from past mistakes that have been identified and have solutions ready. + +Ensure that a verified contract satisfies the following high-level requirements: +* Contract follows a tested and stable Vault standard, +* Stores user funds securely, +* Vulnerabilities identified in various Vault implementations have been taken into account during implementation. + +Category “C4” lists requirements related to the Vault smart contract as one of the project components. + +## Security Verification Requirements + +| # | Description | +| --- | --- | +| **C4.1** | Verify that there are no vulnerabilities associated with the Vault component. | +| **C4.2** | Verify that the user's balance is updated based on funds received and not the funds declared to be sent. | +| **C4.3** | Verify that only the user has access to their funds unless they explicitly allow their funds to be managed. | +| **C4.4** | Verify that the user can safely withdraw their funds at any time, even during an emergency pause. | +| **C4.5** | Verify that if there is a Vault in the system, then users' funds are stored on it. | +| **C4.6** | Verify that shares are distributed in proportion to the user's deposited funds. | +| **C4.7** | Verify that the same amount of shares for different users allows the same amount of funds to be withdrawn. | +| **C4.8** | Verify that a user depositing the same amount earlier will get more rewards than the one who deposits the same amount later. | +| **C4.9** | Verify that depositing rounds down the assets and withdrawing rounds up. | +| **C4.10** | Verify that the transferred amount is confirmed by checking balances before and after the deposit if the vault is going to support fee-on-transfer tokens (or any token is allowed). | +| **C4.11** | Verify that the vault is allowed to transfer tokens only from *msg.sender* to prohibit stealing from users who approved the vault contract. | + +| **C4.12** | Verify that the deposit and withdraw business logic is consistent and symmetrical, especially when re-sending tokens to the same address (*from* == *to*). | + + +## References + +For more information, see also: + +* [Vault | Solidity 0.8](https://www.youtube.com/watch?v=HHoa0c3AOqo) +* [EIP-4626](https://eips.ethereum.org/EIPS/eip-4626) +* [ERC4626 Tokenized Vault Standard](https://academy.apeworx.io/articles/erc-4626-tokenized-vault-standard) +* [Securing ERC4626 Implementations](https://www.youtube.com/watch?v=5KVD7EX6HWQ) +* [Yearn Finance Vault Audit](https://github.com/yearn/yearn-security/blob/master/audits/20210719_ToB_yearn_vaultsv2/ToB_-_Yearn_Vault_v_2_Smart_Contracts_Audit_Report.pdf) diff --git a/2.0/0x200-Components/0x205-C5-Bridge.md b/2.0/0x200-Components/0x205-C5-Bridge.md new file mode 100644 index 0000000..f6e6e04 --- /dev/null +++ b/2.0/0x200-Components/0x205-C5-Bridge.md @@ -0,0 +1,45 @@ +# C5: Bridge + +## Control Objective + +If a project contains a Bridge smart contract, it is necessary to follow the standard and create a secure contract based on it. Learn from past mistakes that have been identified and have solutions ready. + +Ensure that a verified contract satisfies the following high-level requirements: +* Contract follows a tested and stable Bridge standard, +* It is impossible to abuse the signature verification logic, +* Potential threats related to bridges are taken into consideration. + +Category “C5” lists requirements related to the Bridge smart contract as one of the project components. + +## Security Verification Requirements + +| # | Description | +| --- | --- | +| **C5.1** | Verify that bridge requires all necessary values to be included in the message and signed: chain ids, receiver, amount, nonce. | +| **C5.2** | Verify that used signatures are invalidated to protect the bridge from replay attacks. | +| **C5.3** | Verify that the message hash generation algorithm is resistant to collision attacks. | +| **C5.4** | Verify that bridge includes source and destination chain identifiers in the signed message and correctly verifies them. | +| **C5.5** | Verify that bridge does not allow spoofing chain identifiers. | +| **C5.6** | Verify that bridge uses a nonce parameter to allow the same operation (the same sender, receiver, and amount) to be executed multiple times. | +| **C5.7** | Verify that signed message cannot be used in a different context (use domain separator from EIP-712). | +| **C5.8** | Verify that the case of 0 being returned by ecrecover function is handled securely. | +| **C5.9** | Verify that privileged contracts are separated from cross-chain relay calls. Attacker should not be able to cross-call the privileged contract. | +| **C5.11** | Verify each supported blockchain's finality is taken into account when settling relay calls. | +| **C5.12** | Verify that bridge disregards calls originating from different function than designed. | +| **C5.13** | Verify that bridge requires adequate amount of fees to process the message. | +| **C5.14** | Verify that the maximum gas consumption for relayed messages is limited or fully backed by sender (e.g., in terms of fee). | + +## References +For more information, see also: +* [EEA Crosschain Security Guidelines Version 1.0](https://entethalliance.github.io/crosschain-interoperability/crosschainsecurityguidelines.html) +* [Open problems in cross-chain protocols](https://arxiv.org/pdf/2101.12412.pdf) +* [Cross Chain Awareness](https://docs.openzeppelin.com/contracts/4.x/api/crosschain) +* [The Dark Side of DeFi: Cross-Chain Bridge Hacks](https://quantstamp.com/blog/the-dark-side-of-defi-cross-chain-bridge-hacks/) +* [What Are Cross-Chain Smart Contracts?](https://blog.chain.link/cross-chain-smart-contracts/) +* [Aurora Inflation Spend Bugfix Review](https://medium.com/immunefi/aurora-infinite-spend-bugfix-review-6m-payout-e635d24273d) +* [WORMHOLE - REKT](https://rekt.news/wormhole-rekt/) +* [RONIN NETWORK - REKT](https://rekt.news/ronin-rekt/) +* [EIP-712: Ethereum typed structured data hashing and signing](https://eips.ethereum.org/EIPS/eip-712) +* [Learn EVM attacks - Bridges](https://github.com/coinspect/learn-evm-attacks#bridges) +* [Awesome Interoperability - bridges](https://docs.nomad.xyz/resources/awesome-interoperability) +* [PolyNetwork hack - privilege escalation via cross-chain call](https://research.kudelskisecurity.com/2021/08/12/the-poly-network-hack-explained/) diff --git a/2.0/0x200-Components/0x205-C5-Liquidity-pool.md b/2.0/0x200-Components/0x205-C5-Liquidity-pool.md deleted file mode 100644 index e69de29..0000000 diff --git a/2.0/0x200-Components/0x206-C6-Bridge.md b/2.0/0x200-Components/0x206-C6-Bridge.md deleted file mode 100644 index 8844d68..0000000 --- a/2.0/0x200-Components/0x206-C6-Bridge.md +++ /dev/null @@ -1,36 +0,0 @@ -# C6: Bridge - -## Control Objective - -If a project contains a Bridge smart contract, it is necessary to follow the standard and create a secure contract based on it. Learn from past mistakes that have been identified and have solutions ready. - -Ensure that a verified contract satisfies the following high-level requirements: -* Contract follows a tested and stable Bridge standard, -* It is impossible to abuse the signature verification logic, -* Potential threats related to bridges are taken into consideration. - -Category “C6” lists requirements related to the Bridge smart contract as one of the project components. - -## Security Verification Requirements - -| # | Description | -| --- | --- | -| **C6.1** | Verify that bridge requires all necessary values to be included in the message and signed: chain ids, receiver, amount, nonce. | -| **C6.2** | Verify that used signatures are invalidated to protect bridge from replay attacks. | -| **C6.3** | Verify that message hash generation algorithm is resistant to collision attacks. | -| **C6.4** | Verify that bridge includes source and destination chains identifiers in the signed message and correctly verifies them. | -| **C6.5** | Verify that bridge does not allow to spoof chain identifier. | -| **C6.6** | Verify that bridge uses a nonce parameter to allow the same operation (the same sender, receiver and amount) to be executed multiple times. | -| **C6.7** | Verify signed message cannot be used in a differenct context (use domain separator from EIP-712). | - -## References -For more information, see also: -* [EEA Crosschain Security Guidelines Version 1.0](https://entethalliance.github.io/crosschain-interoperability/crosschainsecurityguidelines.html) -* [Open problems in cross-chain protocols](https://arxiv.org/pdf/2101.12412.pdf) -* [Cross Chain Awareness](https://docs.openzeppelin.com/contracts/4.x/api/crosschain) -* [The Dark Side of DeFi: Cross-Chain Bridge Hacks](https://quantstamp.com/blog/the-dark-side-of-defi-cross-chain-bridge-hacks/) -* [What Are Cross-Chain Smart Contracts?](https://blog.chain.link/cross-chain-smart-contracts/) -* [Aurora Inflation Spend Bugfix Review](https://medium.com/immunefi/aurora-infinite-spend-bugfix-review-6m-payout-e635d24273d) -* [WORMHOLE - REKT](https://rekt.news/wormhole-rekt/) -* [RONIN NETWORK - REKT](https://rekt.news/ronin-rekt/) -* [EIP-712: Ethereum typed structured data hashing and signing](https://eips.ethereum.org/EIPS/eip-712) \ No newline at end of file diff --git a/2.0/0x300-Integrations/0x301-I1-Basic.md b/2.0/0x300-Integrations/0x301-I1-Basic.md index 7a9596d..22347fd 100644 --- a/2.0/0x300-Integrations/0x301-I1-Basic.md +++ b/2.0/0x300-Integrations/0x301-I1-Basic.md @@ -2,9 +2,9 @@ ## Control Objective -The spirit of composability introduce many integrations between different entities, creating complex and multi-level relationships at the same time. This environment can introduce complex logical vulnerabilities or issues related to bad assumptions due to too much trust in external components. +The spirit of composability introduces many integrations between different entities, creating complex and multi-level relationships at the same time. This environment can introduce complex logical vulnerabilities or issues related to bad assumptions due to too much trust in external components. -Ensure that integration process satisfies the following high-level requirements: +Ensure that the integration process satisfies the following high-level requirements: * We integrate with a proven component that we consider trusted, * Vulnerabilities identified in various Token implementations have been taken into account during implementation. @@ -14,22 +14,22 @@ Category “I1” lists requirements related to each integration with any smart | # | Description | | --- | --- | -| **I1.1** | Verify that there are no vulnerabilities associated with Basic category. | +| **I1.1** | Verify that there are no vulnerabilities associated with the Basic category. | | **I1.2** | Verify if the team is known and can be held liable for abuses. | | **I1.3** | Verify that the external contract you want to use has been audited. | | **I1.4** | Verify that the external contract you want to use has been verified on blockchain explorer (e.g., etherscan). | | **I1.5** | Verify that the contract with which you are integrating has passed SCSVS. | -| **I1.6** | Verify that the address of the integrated contract is correct, up-to-date and consistent with the project documentation. | +| **I1.6** | Verify that the address of the integrated contract is correct, up-to-date, and consistent with the project documentation. | | **I1.7** | Verify if the external contract is upgradeable in a trusted manner. | | **I1.8** | Verify that no untrusted party can change the contract's implementation. | | **I1.9** | Verify that external contracts comply with the principle of minimum rights and that their access to special functions is enforced by the appropriate modifiers. | | **I1.10** | Verify whether the structures (types) received from the external contract are known and handled. | | **I1.11** | Verify that the smart contract attributes that can be updated by the external contracts (even trusted) are monitored (e.g. using events) and the procedure of incident response is implemented (e.g. the response to an ongoing attack). | | **I1.12** | Verify that the external contracts (even trusted) that are allowed to change the attributes of the smart contract (e.g., token price) have the following limitations implemented: a threshold for the change (e.g. no more/less than 5%) and a limit of updates (e.g., one update per day). | -| **I1.13** | Verify that the address called via low-level call/delegatecall/staticcall exists (it will return true if the contract does not exist). | +| **I1.13** | Verify that the address called via low-level call/delegatecall/staticcall exists (it will return *true* if the contract does not exist). | ## References For more information, see also: -* [TODO](https://securing.biz/) +* TODO diff --git a/2.0/0x300-Integrations/0x302-I2-Token.md b/2.0/0x300-Integrations/0x302-I2-Token.md index 919cfef..32cef44 100644 --- a/2.0/0x300-Integrations/0x302-I2-Token.md +++ b/2.0/0x300-Integrations/0x302-I2-Token.md @@ -17,17 +17,19 @@ Category “I2” lists requirements related to the Token smart contract as one | --- | --- | | **I2.1** | Verify that there are no vulnerabilities associated with Token integrations. | | **I2.2** | Verify if the external Token implementation is compliant with the standard implementation. | -| **I2.3** | Verify if the rules on which a new external Token can be added to the system have been defined (no restrictions, any tokens added by Governance etc.). | +| **I2.3** | Verify if the rules on which a new external Token can be added to the system have been defined (no restrictions, any tokens added by Governance, etc.). | | **I2.4** | Verify that the allowlist approach is used when only selected tokens are introduced to the system. | -| **I2.5** | Verify if the external Token implementation is non-standard (e.g. it is deflationary, contains a fee), it has been taken into consideration. | +| **I2.5** | Verify if the external Token implementation is non-standard (e.g. it is deflationary, or contains a fee), it has been taken into consideration. | | **I2.6** | Verify that if the external Token implementation includes external calls, it has been taken into consideration (e.g., protection against reentrancy). | -| **I2.7** | Verify that the external Token magnitude (decimals) are known, and all operations are executed with the correct magnitude. | +| **I2.7** | Verify that the external Token magnitude (decimals) are known and that all operations are executed with the correct magnitude. | | **I2.8** | Verify that the external Token supply is specified and corresponds to the documentation. | | **I2.9** | Verify that the external Tokens of any user cannot be locked or frozen by any entity (e.g., owner). | | **I2.10** | Verify that the reentrancy attack has been considered when using the token contracts with callbacks (e.g. ERC-777, ERC-721, ERC-1155). | | **I2.11** | Verify that transfer of external Tokens has been successful, comparing the balances before and after it. | -| **I2.12** | Verify that projects contracts handles correctly both types of tokens, those that return false on error and those that revert. | +| **I2.12** | Verify that project contracts handles correctly both types of tokens, those that return false on an error and those that revert. | | **I2.13** | Verify that the contract reverts on failed transfer. | +| **I2.14** | Verify that the protocol handles double-entry tokens (tracking user balances in a contract represented by two addresses) correctly or forbids them. | +| **I2.15** | Use OpenZeppelin's SafeERC20 for interacting with ERC20 tokens. | ## References @@ -38,4 +40,5 @@ For more information, see also: * [The Dangers of Token Integration](https://www.youtube.com/watch?v=6GaCt_lM_ak) * [Token Implementation Best Practice](https://consensys.github.io/smart-contract-best-practices/tokens/) * [iToken Duplication Incident Report](https://bzx.network/blog/incident) -* [The Dangers of Surprising Code](https://samczsun.com/the-dangers-of-surprising-code/) \ No newline at end of file +* [The Dangers of Surprising Code](https://samczsun.com/the-dangers-of-surprising-code/) +* [ERC20 standard peculiarities](https://github.com/d-xo/weird-erc20) diff --git a/2.0/0x300-Integrations/0x303-I3-Governance.md b/2.0/0x300-Integrations/0x303-I3-Governance.md deleted file mode 100644 index e69de29..0000000 diff --git a/2.0/0x300-Integrations/0x303-I3-Oracle.md b/2.0/0x300-Integrations/0x303-I3-Oracle.md new file mode 100644 index 0000000..05122f7 --- /dev/null +++ b/2.0/0x300-Integrations/0x303-I3-Oracle.md @@ -0,0 +1,34 @@ +# I3: Oracle + +## Control Objective + +If a project integrates with external Oracle smart contracts, it is necessary to approach them with limited trust and check that they do not introduce unexpected behavior into our system. + +Ensure that a verified contract satisfies the following high-level requirements: +* Contract follows a tested and stable Oracle standard, +* The values transferred are additionally verified, +* Vulnerabilities identified in various Oracle implementations have been taken into account during implementation. + +Category “I3” lists requirements related to the Oracle smart contract as one of the components with which the project integrates. + +## Security Verification Requirements + +| # | Description | +| --- | --- | +| **I3.1** | Verify that there are no vulnerabilities associated with Oracle integration. | +| **I3.2** | Verify that, when using Uniswap TWAP as a price oracle, the period is long enough to make its manipulation unprofitable for the attacker (compared to the funds at potential risk). | +| **I3.3** | Verify that Oracle data is up-to-date. | +| **I3.4** | Verify that no spot oracle is used (e.g. spot price from Uniswap pool). | +| **I4.4** | Verify that, when using Uniswap V3 TWAP as price oracle, liquidity is high enough and is distributed widely across most of the price range. | +| **I4.5** | Verify that, the use a decentralized off-chain oracles unsusceptible to on-chain price manipulation attacks (e.g. Chainlink) is considered for low liquidity asset, ideally combining it with on-chain oracles to detect malicious values. | + +## References + +For more information, see also: + +* [The Dangers of Price Oracles in Smart Contracts](https://www.youtube.com/watch?v=YGO7nzpXCeA) +* [TWAP Oracle Manipulation Risks, Mudit Gupta - DeFi Security Summit 2022](https://www.youtube.com/watch?v=Mu8ytTyStOU) +* [TWAP Oracles After the Merge, Mark Toda - DeFi Security Summit 2022](https://www.youtube.com/watch?v=mqrCvNgBXUk) +* [So you want to use a price oracle](https://samczsun.com/so-you-want-to-use-a-price-oracle/) +* [Pricing LP tokens | Warp Finance hack](https://cmichel.io/pricing-lp-tokens/) +* [Uniswap V3 tick price manipulation](https://medium.com/@hacxyk/we-rescued-4m-from-rari-capital-but-was-it-worth-it-39366d4d1812) diff --git a/2.0/0x300-Integrations/0x304-I4-Oracle.md b/2.0/0x300-Integrations/0x304-I4-Oracle.md deleted file mode 100644 index f50e300..0000000 --- a/2.0/0x300-Integrations/0x304-I4-Oracle.md +++ /dev/null @@ -1,23 +0,0 @@ -# I4: Oracle - -## Control Objective - -TODO - -Category “I4” lists requirements related to the Oracle smart contract as one of the components with which the project integrates. - -## Security Verification Requirements - -| # | Description | -| --- | --- | -| **I4.1** | Verify that there are no vulnerabilities associated with Oracle integration. | -| **I4.2** | Verify that, when using Uniswap TWAP as price oracle, the time period is long enough to make its manipulation unprofitable for the attacker (compared to the funds at potential risk). | -| **I4.3** | Verify that Oracle data is up-to-date. | - -## References - -For more information, see also: - -* [The Dangers of Price Oracles in Smart Contracts](https://www.youtube.com/watch?v=YGO7nzpXCeA) -* [So you want to use a price oracle](https://samczsun.com/so-you-want-to-use-a-price-oracle/) -* [Pricing LP tokens | Warp Finance hack](https://cmichel.io/pricing-lp-tokens/) \ No newline at end of file diff --git a/2.0/0x300-Integrations/0x305-I5-Flash-loan-provider.md b/2.0/0x300-Integrations/0x305-I5-Flash-loan-provider.md deleted file mode 100644 index e69de29..0000000 diff --git a/2.0/0x300-Integrations/0x306-I6-Liquidity-pool.md b/2.0/0x300-Integrations/0x306-I6-Liquidity-pool.md deleted file mode 100644 index e69de29..0000000 diff --git a/README.md b/README.md index 76bbb46..689f046 100644 --- a/README.md +++ b/README.md @@ -2,29 +2,27 @@ ## Authors -* Damian Rusinek [@drdr_zz](https://twitter.com/drdr_zz) (damian.rusinek@securing.pl) -* Paweł Kuryłowicz [@wh01s7](https://twitter.com/wh01s7) (pawel.kurylowicz@securing.pl) +* Damian Rusinek [@drdr_zz](https://twitter.com/drdr_zz) (damian.rusinek@composable-security.com) +* Paweł Kuryłowicz [@wh01s7](https://twitter.com/wh01s7) (pawel.kurylowicz@composable-security.com) ## Introduction -Smart Contract Security Verification Standard (v2) is a FREE checklist created to **standardize the security of smart contracts** for developers, architects, security reviewers and vendors. +Smart Contract Security Verification Standard (v2) is a FREE checklist created to **standardize the security of smart contracts** for developers, architects, security reviewers, and vendors. -This list helps to avoid the majority of known security problems and vulnerabilities by providing guidance at every stage of the development cycle of the smart contracts (from designing to implementation). +This list helps to avoid the majority of known security problems and vulnerabilities by guiding at every stage of the development cycle of smart contracts (from design to implementation). **Objectives** -* Help to develop high quality code of the smart contracts. +* Help to develop high-quality code of smart contracts. * Help to mitigate known vulnerabilities by design. * Provide a checklist for security reviewers. * Provide a clear and reliable assessment - **Security Health Factor** - of how secure smart contracts are in the relation to the percentage of SCSVS coverage. ## 🔥 Updates in v2 🔥 -Security, Composability and Transparency are fundamentals of the SCSVS. These values are achieved thanks to the engagement and cooperation of the #BlockSec community. The standard structure distinguishes 3 chapters, each operating in a slightly different area. - -* **General** - common and general security problems including, among others, design, upgrades, policies. +Security, Composability, and Transparency are fundamentals of the SCSVS. These values are achieved thanks to the engagement and cooperation of the #BlockSec community. The standard structure distinguishes 3 chapters, each operating in a slightly different area. +* **General** - common and general security problems including, among others: design, upgrades, and policies. * **Components** - contracts that make up the project, frequently used patterns with their typical security issues. - * **Integrations** - components with which the project integrates, general recommendations and threats to frequently used smart contracts. ## How to use SCSVSv2 @@ -32,18 +30,18 @@ Security, Composability and Transparency are fundamentals of the SCSVS. These va You can use the SCSVS checklist in multiple ways: * As a starting point for formal threat modeling exercise. * As a measure of your smart contract security and maturity. -* As a scoping document for penetration test or security audit of a smart contract. +* As a scoping document for a penetration test or security audit of a smart contract. * As a formal security requirement list for developers or third parties developing the smart contract for you. * As a self-check for developers. -* To point areas which need further development regarding security. +* To point out areas that need further development regarding security. Depending on your role in the protocol, we recommend performing different actions summarized below. Treat it as an inspiration and not strict rules. ### As Architect 👷‍♂️ If you are designing a protocol, you should schedule (and potentially delegate) the following actions: -* Use security requirements from General and Components chapters to identify potential security bugs your team might introduce in the protocol. -* Use security requirements from Interaction chapter to identify potential threat coming from the 3rd party protocols. +* Use security requirements from the General and Components chapters to identify potential security bugs your team might introduce in the protocol. +* Use security requirements from the Interaction chapter to identify potential threats coming from the 3rd party protocols. * Write down all potential threats identified in steps 1 and 2. This will be great input for the threat modeling session. * Inform your team about the identified threats (e.g. during a threat modeling session). @@ -52,12 +50,12 @@ If you are designing a protocol, you should schedule (and potentially delegate) If you are developing a protocol, you should schedule (and potentially delegate) the following actions: * Get the potential threats identified by the Auditor, Architect or during a threat modeling session and learn how to avoid them. * When implementing a smart contract with specific functionalities, check the security requirements from the corresponding SCSVS category. -* After release, make sure SCSVS coverage report is present in the protocol repository. +* After release, make sure the SCSVS coverage report is present in the protocol repository. ### As Business Owner / Founder 🧙 If you are the owner of a protocol or represent the business side of the project, you should schedule and delegate the following actions: -* Ask other protocols that yours is integrated with for their SCSVS coverage report. +* Ask other protocols teams that yours is integrated with for their SCSVS coverage report. * Check the **Security Health Factor** of your protocol by doing SCSVS coverage of all 3 chapters. ### As Auditor 🥷 @@ -88,25 +86,21 @@ If you are an internal or external auditor of a protocol, you should schedule (a * [C2: Governance](<./2.0/0x200-Components/0x202-C2-Governance.md>) * [C3: Oracle](<./2.0/0x200-Components/0x203-C3-Oracle.md>) * [C4: Vault](<./2.0/0x200-Components/0x204-C4-Vault.md>) - * [C5: Liquidity pool](<./2.0/0x200-Components/0x205-C5-Liquidity-pool.md>) - * [C6: Bridge](<./2.0/0x200-Components/0x206-C6-Bridge.md>) + * [C5: Bridge](<./2.0/0x200-Components/0x205-C5-Bridge.md>) * I: Integrations * [I1: Basic](<./2.0/0x300-Integrations/0x301-I1-Basic.md>) * [I2: Token](<./2.0/0x300-Integrations/0x302-I2-Token.md>) - * [I3: Governance](<./2.0/0x300-Integrations/0x303-I3-Governance.md>) - * [I4: Oracle](<./2.0/0x300-Integrations/0x304-I4-Oracle.md>) - * [I5: Flash loan provider](<./2.0/0x300-Integrations//0x305-I5-Flash-loan-provider.md>) - * [I6: Liquidity pool](<./2.0/0x300-Integrations/0x306-I6-Liquidity-pool.md>) + * [I3: Oracle](<./2.0/0x300-Integrations/0x303-I3-Oracle.md>) ## Severity of the risk -Threat modeling and risk analysis are important parts of the security assessment. Threat modeling allows discovering potential threats and their risk impact. The aim of risk analysis is to identify security risks and determine their severity, which allows the team to prioritize them in the mitigation process. +Threat modeling and risk analysis are important parts of the security assessment. Threat modeling allows the discovery of potential threats and their risk impact. Risk analysis aims to identify security risks and determine their severity, which allows the team to prioritize them in the mitigation process. The SCSVS does not include the severity of the risks related to the requirements. Even though there are multiple methodologies to assess the severity, each application is unique and so are the threat actors, their goals, and the impact of a breach. Moreover, the requirements cannot be uniquely linked to the security risks, as many risks can refer to one requirement and many requirements can refer to one risk. -We recommend determining the severity of the risks related with the requirements when performing the security assessment using SCSVS standard. +We recommend determining the severity of the risks related to the requirements when performing the security assessment using the SCSVS standard. We recommend [Common Vulnerability Scoring System (CVSS)](https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator), a free and open industry standard for assessing the severity of security vulnerabilities. @@ -114,4 +108,4 @@ We recommend [Common Vulnerability Scoring System (CVSS)](https://nvd.nist.gov/v This work is licensed under the Creative Commons Attribution-ShareAlike 4.0 International License. To view a copy of this license, visit http://creativecommons.org/licenses/by-sa/4.0/ or send a letter to Creative Commons, PO Box 1866, Mountain View, CA 94042, USA. The entire checklist is in a form similar to OWASP APPLICATION SECURITY VERIFICATION STANDARD v4.0. -Every category has a brief description of the control objectives and a list of security verification requirements. \ No newline at end of file +Every category has a brief description of the control objectives and a list of security verification requirements. diff --git a/SCSVS_v1.1.pdf b/SCSVS_v1.1.pdf deleted file mode 100644 index cd8f9e9..0000000 Binary files a/SCSVS_v1.1.pdf and /dev/null differ