From ee57e111168b2c806f91a71b2fbde1f6f8865cf6 Mon Sep 17 00:00:00 2001 From: broccolirob Date: Fri, 1 Sep 2023 02:08:59 -0400 Subject: [PATCH 1/8] Preparing for a security review checklist --- development-guidelines/README.md | 1 + development-guidelines/review_checklist.md | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 development-guidelines/review_checklist.md diff --git a/development-guidelines/README.md b/development-guidelines/README.md index cccecc5a..bd35a44f 100644 --- a/development-guidelines/README.md +++ b/development-guidelines/README.md @@ -5,3 +5,4 @@ List of Best Practices for Smart Contract Development - [Token Integration Checklist](./token_integration.md): Important aspects to consider when interacting with various tokens - [Incident Response Recommendations](./incident_response.md): Guidelines on establishing an effective incident response plan - [Secure Development Workflow](./workflow.md): A recommended high-level process to adhere to while writing code +- [Preparing for a Security Review](./review_checklist.md): A checklist of things to consider when preparing for a security review diff --git a/development-guidelines/review_checklist.md b/development-guidelines/review_checklist.md new file mode 100644 index 00000000..4441aaba --- /dev/null +++ b/development-guidelines/review_checklist.md @@ -0,0 +1,18 @@ +# Preparing for a Security Review Checklist +Here’s how to make that security review more effective, valuable, and satisfying for everybody involved. + +### Resolve the Easy Issues +- [] **Enable and address compiler warnings.** Go after the easy stuff first: turn on every single compiler warning you can find, understand each warning, then fix your code until they’re all gone. Upgrade your compiler to the latest version, then fix all the new warnings and errors. Even innocuous seeming warnings can indicate problems lying in wait. +- [] **Increase unit and feature test coverage.** Ideally this has been part of the development process, but everyone slips up, tests don’t get updated, or new features don’t quite match the old integrations tests. Now is the time to update the tests and run them all. +- [] **Remove dead code, stale branches, unused libraries, and other extraneous weight.** You may know which branch is active and which is dead but the consultants won’t and will waste time investigating it for potential issues. The same goes for that new feature that hasn’t seen progress in months, or that third-party library that doesn’t get used anymore. + +### Document +- [] **Provide external documentation and reports from any prior security reviews.** +- [] **Describe what your product does, who uses it, and how.** The most important documentation is high level: what does your product do? What do users want from it? How does it achieve that goal? Use clear language to describe how systems interact and the rationale for design decisions made during development. +- [] **Add comments in-line with the code. Preferably in NatSpec format.** Functions should have comments containing high-level descriptions of their intended behavior. Complicated sections of code should have comments describing what is happening and why this particular approach was chosen. +- [] **Label and describe your tests.** More complicated tests should describe the exact behavior they’re testing. The expected results of tests, both positive and negative, should be documented. + +### Deliver the Code, Batteries Included +- [] **Prepare the build environment.** Document the steps to create a build environment from scratch on a computer that is fully disconnected from your internal network. Where relevant, be specific about software versions. Walk through this process on your own to ensure that it is complete. If you have external dependencies that are not publicly available, include them with your code. Fully provisioned virtual machine images are a great way to deliver a working build environment. +- [] **Document the build process.** Include both the debug and release build processes, and also include steps on how to build and run the tests. If the test environment is distinct from the build environment, include steps on how to create the test environment. A well-documented build process enables a consultant to run static analysis tools far more efficiently and effectively. +- [] **Document the deploy process.** This includes how to build the deployment environment. It is very important to list all the specific versions of external tools and libraries for this process, as the deployment environment is a considerable factor in evaluating the security of your product. A well-documented deployment process enables a consultant to run dynamic analysis tools in a real world environment. \ No newline at end of file From f538de7eefb12283e09afb4ee6ca6353ab25426e Mon Sep 17 00:00:00 2001 From: broccolirob Date: Fri, 8 Sep 2023 10:01:13 -0400 Subject: [PATCH 2/8] Fix lint error --- development-guidelines/review_checklist.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/development-guidelines/review_checklist.md b/development-guidelines/review_checklist.md index 4441aaba..96ed17c8 100644 --- a/development-guidelines/review_checklist.md +++ b/development-guidelines/review_checklist.md @@ -1,18 +1,22 @@ # Preparing for a Security Review Checklist + Here’s how to make that security review more effective, valuable, and satisfying for everybody involved. ### Resolve the Easy Issues + - [] **Enable and address compiler warnings.** Go after the easy stuff first: turn on every single compiler warning you can find, understand each warning, then fix your code until they’re all gone. Upgrade your compiler to the latest version, then fix all the new warnings and errors. Even innocuous seeming warnings can indicate problems lying in wait. - [] **Increase unit and feature test coverage.** Ideally this has been part of the development process, but everyone slips up, tests don’t get updated, or new features don’t quite match the old integrations tests. Now is the time to update the tests and run them all. - [] **Remove dead code, stale branches, unused libraries, and other extraneous weight.** You may know which branch is active and which is dead but the consultants won’t and will waste time investigating it for potential issues. The same goes for that new feature that hasn’t seen progress in months, or that third-party library that doesn’t get used anymore. ### Document + - [] **Provide external documentation and reports from any prior security reviews.** - [] **Describe what your product does, who uses it, and how.** The most important documentation is high level: what does your product do? What do users want from it? How does it achieve that goal? Use clear language to describe how systems interact and the rationale for design decisions made during development. - [] **Add comments in-line with the code. Preferably in NatSpec format.** Functions should have comments containing high-level descriptions of their intended behavior. Complicated sections of code should have comments describing what is happening and why this particular approach was chosen. - [] **Label and describe your tests.** More complicated tests should describe the exact behavior they’re testing. The expected results of tests, both positive and negative, should be documented. ### Deliver the Code, Batteries Included + - [] **Prepare the build environment.** Document the steps to create a build environment from scratch on a computer that is fully disconnected from your internal network. Where relevant, be specific about software versions. Walk through this process on your own to ensure that it is complete. If you have external dependencies that are not publicly available, include them with your code. Fully provisioned virtual machine images are a great way to deliver a working build environment. - [] **Document the build process.** Include both the debug and release build processes, and also include steps on how to build and run the tests. If the test environment is distinct from the build environment, include steps on how to create the test environment. A well-documented build process enables a consultant to run static analysis tools far more efficiently and effectively. -- [] **Document the deploy process.** This includes how to build the deployment environment. It is very important to list all the specific versions of external tools and libraries for this process, as the deployment environment is a considerable factor in evaluating the security of your product. A well-documented deployment process enables a consultant to run dynamic analysis tools in a real world environment. \ No newline at end of file +- [] **Document the deploy process.** This includes how to build the deployment environment. It is very important to list all the specific versions of external tools and libraries for this process, as the deployment environment is a considerable factor in evaluating the security of your product. A well-documented deployment process enables a consultant to run dynamic analysis tools in a real world environment. From ae25a41433e574bdffd2b20d8e5504713e142d94 Mon Sep 17 00:00:00 2001 From: broccolirob Date: Mon, 25 Sep 2023 11:54:50 -0400 Subject: [PATCH 3/8] Fix checkboxes so they appear correctly in markdown --- development-guidelines/review_checklist.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/development-guidelines/review_checklist.md b/development-guidelines/review_checklist.md index 96ed17c8..2f8cd72a 100644 --- a/development-guidelines/review_checklist.md +++ b/development-guidelines/review_checklist.md @@ -4,19 +4,19 @@ Here’s how to make that security review more effective, valuable, and satisfyi ### Resolve the Easy Issues -- [] **Enable and address compiler warnings.** Go after the easy stuff first: turn on every single compiler warning you can find, understand each warning, then fix your code until they’re all gone. Upgrade your compiler to the latest version, then fix all the new warnings and errors. Even innocuous seeming warnings can indicate problems lying in wait. -- [] **Increase unit and feature test coverage.** Ideally this has been part of the development process, but everyone slips up, tests don’t get updated, or new features don’t quite match the old integrations tests. Now is the time to update the tests and run them all. -- [] **Remove dead code, stale branches, unused libraries, and other extraneous weight.** You may know which branch is active and which is dead but the consultants won’t and will waste time investigating it for potential issues. The same goes for that new feature that hasn’t seen progress in months, or that third-party library that doesn’t get used anymore. +- [ ] **Enable and address compiler warnings.** Go after the easy stuff first: turn on every single compiler warning you can find, understand each warning, then fix your code until they’re all gone. Upgrade your compiler to the latest version, then fix all the new warnings and errors. Even innocuous seeming warnings can indicate problems lying in wait. +- [ ] **Increase unit and feature test coverage.** Ideally this has been part of the development process, but everyone slips up, tests don’t get updated, or new features don’t quite match the old integrations tests. Now is the time to update the tests and run them all. +- [ ] **Remove dead code, stale branches, unused libraries, and other extraneous weight.** You may know which branch is active and which is dead but the consultants won’t and will waste time investigating it for potential issues. The same goes for that new feature that hasn’t seen progress in months, or that third-party library that doesn’t get used anymore. ### Document -- [] **Provide external documentation and reports from any prior security reviews.** -- [] **Describe what your product does, who uses it, and how.** The most important documentation is high level: what does your product do? What do users want from it? How does it achieve that goal? Use clear language to describe how systems interact and the rationale for design decisions made during development. -- [] **Add comments in-line with the code. Preferably in NatSpec format.** Functions should have comments containing high-level descriptions of their intended behavior. Complicated sections of code should have comments describing what is happening and why this particular approach was chosen. -- [] **Label and describe your tests.** More complicated tests should describe the exact behavior they’re testing. The expected results of tests, both positive and negative, should be documented. +- [ ] **Provide external documentation and reports from any prior security reviews.** Including external documentation and reports from previous security reviews is important for gaining a comprehensive understanding of the system, identifying past issues, and reviewing any remediation measures taken. This information is crucial for conducting a thorough and effective security audit. +- [ ] **Describe what your product does, who uses it, and how.** The most important documentation is high level: what does your product do? What do users want from it? How does it achieve that goal? Use clear language to describe how systems interact and the rationale for design decisions made during development. +- [ ] **Add comments in-line with the code. Preferably in NatSpec format.** Functions should have comments containing high-level descriptions of their intended behavior. Complicated sections of code should have comments describing what is happening and why this particular approach was chosen. +- [ ] **Label and describe your tests.** More complicated tests should describe the exact behavior they’re testing. The expected results of tests, both positive and negative, should be documented. ### Deliver the Code, Batteries Included -- [] **Prepare the build environment.** Document the steps to create a build environment from scratch on a computer that is fully disconnected from your internal network. Where relevant, be specific about software versions. Walk through this process on your own to ensure that it is complete. If you have external dependencies that are not publicly available, include them with your code. Fully provisioned virtual machine images are a great way to deliver a working build environment. -- [] **Document the build process.** Include both the debug and release build processes, and also include steps on how to build and run the tests. If the test environment is distinct from the build environment, include steps on how to create the test environment. A well-documented build process enables a consultant to run static analysis tools far more efficiently and effectively. -- [] **Document the deploy process.** This includes how to build the deployment environment. It is very important to list all the specific versions of external tools and libraries for this process, as the deployment environment is a considerable factor in evaluating the security of your product. A well-documented deployment process enables a consultant to run dynamic analysis tools in a real world environment. +- [ ] **Prepare the build environment.** Document the steps to create a build environment from scratch on a computer that is fully disconnected from your internal network. Where relevant, be specific about software versions. Walk through this process on your own to ensure that it is complete. If you have external dependencies that are not publicly available, include them with your code. Fully provisioned virtual machine images are a great way to deliver a working build environment. +- [ ] **Document the build process.** Include both the debug and release build processes, and also include steps on how to build and run the tests. If the test environment is distinct from the build environment, include steps on how to create the test environment. A well-documented build process enables a consultant to run static analysis tools far more efficiently and effectively. +- [ ] **Document the deploy process.** This includes how to build the deployment environment. It is very important to list all the specific versions of external tools and libraries for this process, as the deployment environment is a considerable factor in evaluating the security of your product. A well-documented deployment process enables a consultant to run dynamic analysis tools in a real world environment. From 7f66b9ed2eef4490434472fb34f83b9791371c1f Mon Sep 17 00:00:00 2001 From: Nat Chin Date: Thu, 7 Dec 2023 13:33:36 -0500 Subject: [PATCH 4/8] Initial push of preparation checklist --- development-guidelines/review_checklist.md | 58 ++++++++++++++++------ 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/development-guidelines/review_checklist.md b/development-guidelines/review_checklist.md index 2f8cd72a..44bab4de 100644 --- a/development-guidelines/review_checklist.md +++ b/development-guidelines/review_checklist.md @@ -1,22 +1,50 @@ -# Preparing for a Security Review Checklist +# Security Review Preparation Checklist -Here’s how to make that security review more effective, valuable, and satisfying for everybody involved. +So, you're getting a security review! -### Resolve the Easy Issues +Ensure the security review proceeds smoothly by taking prior action with the following informative items. -- [ ] **Enable and address compiler warnings.** Go after the easy stuff first: turn on every single compiler warning you can find, understand each warning, then fix your code until they’re all gone. Upgrade your compiler to the latest version, then fix all the new warnings and errors. Even innocuous seeming warnings can indicate problems lying in wait. -- [ ] **Increase unit and feature test coverage.** Ideally this has been part of the development process, but everyone slips up, tests don’t get updated, or new features don’t quite match the old integrations tests. Now is the time to update the tests and run them all. -- [ ] **Remove dead code, stale branches, unused libraries, and other extraneous weight.** You may know which branch is active and which is dead but the consultants won’t and will waste time investigating it for potential issues. The same goes for that new feature that hasn’t seen progress in months, or that third-party library that doesn’t get used anymore. +## Essential Items +The items below guide us in focusing effectively on the pertinent parts of your code, maximizing our utilization of time and resources. -### Document +- [ ] **Provide build instructions** – share how to build and test your code using a fresh clone of your repository. It gives us direct insight into your setup. +- [ ] **List exact files in scope for the review** – list out the exact files to be reviewed to focus on key areas of your codebase. +- [ ] **Freeze and share your hash/branch/release before the review starts** – a stable commit allows us to start digging into your codebase and learning how it works. Tell your security review team what commit to review, and try to ensure that this commit is frozen before the review commences. +- [ ] **Add inline comments** – utilise comments on complex areas of the codebase. Document why something was done, its purpose and goal. +- [ ] **Provide Natspec documentation** – ensure all functions have NatSpec descriptions for function's purpose, parameters, and return values. +- [ ] **Provide test coverage report** – provide a report outlining tested and untested code areas. +- [ ] **Share your unit tests** – share your unit tests with us! We may use it to understand the system flow, can provide feedback on its setup, and can use it to test proof of concepts. +- [ ] **List external dependencies you're using and their purpose** – list used libraries and external dependencies along with their purposes. +- [ ] **Create user flow diagrams** – outline the entrypoint functions, subsequent callstacks, and expected system interactions for users. +- [ ] **Design an architecture diagram** – illustrate the interactions among contracts. +- [ ] **Let us know what you're worried about** – reveal parts of the code or exploit paths that cause concern.This helps us target our review and make sure that we prioritize the areas of the codebase you're worried about. +- [ ] **Document the impossible** – know when you are making assumptions on types and their values into the codebase, and document all these instances. -- [ ] **Provide external documentation and reports from any prior security reviews.** Including external documentation and reports from previous security reviews is important for gaining a comprehensive understanding of the system, identifying past issues, and reviewing any remediation measures taken. This information is crucial for conducting a thorough and effective security audit. -- [ ] **Describe what your product does, who uses it, and how.** The most important documentation is high level: what does your product do? What do users want from it? How does it achieve that goal? Use clear language to describe how systems interact and the rationale for design decisions made during development. -- [ ] **Add comments in-line with the code. Preferably in NatSpec format.** Functions should have comments containing high-level descriptions of their intended behavior. Complicated sections of code should have comments describing what is happening and why this particular approach was chosen. -- [ ] **Label and describe your tests.** More complicated tests should describe the exact behavior they’re testing. The expected results of tests, both positive and negative, should be documented. +### Defi-Specific Additions +- [ ] **Share economic analysis performed on the codebase** – sharing results of economic analysis will help us to understand the boundaries of inputs and ranges of outputs. +- [ ] **List system invariants** – define your assumptions about system operations and function behaviors. +- [ ] **Provide documentation for arithmetic formulas used with code references** – all formulas you implement should be referenced in this document, with a link to the function in your code that implements it. If the representation in your code differs from the formula, provide the derivation that maps the formula used in the code and the formula you intend to implement. +- [ ] **Create a glossary for your system** – consistently used terminology should be documented and made accessible. +- [ ] **Validation of rounding directions** – share all analysis you have done on checking the correct rounding direction for your system. -### Deliver the Code, Batteries Included +### Bridge-Specific Additions +- [ ] **Document off-chain checks** – provide details about validations performed by off-chain components like relayers or bots. +- [ ] **Checks performed by off-chain components such as relayers or bots** tell us what data validation takes place off-chain so we can better understand what data is sent to smart contracts +- [ ] **Transition of value-passing on source and destination chains** – show us how the chains share data, and what return values/emissions from a source chain maps to a destination chain -- [ ] **Prepare the build environment.** Document the steps to create a build environment from scratch on a computer that is fully disconnected from your internal network. Where relevant, be specific about software versions. Walk through this process on your own to ensure that it is complete. If you have external dependencies that are not publicly available, include them with your code. Fully provisioned virtual machine images are a great way to deliver a working build environment. -- [ ] **Document the build process.** Include both the debug and release build processes, and also include steps on how to build and run the tests. If the test environment is distinct from the build environment, include steps on how to create the test environment. A well-documented build process enables a consultant to run static analysis tools far more efficiently and effectively. -- [ ] **Document the deploy process.** This includes how to build the deployment environment. It is very important to list all the specific versions of external tools and libraries for this process, as the deployment environment is a considerable factor in evaluating the security of your product. A well-documented deployment process enables a consultant to run dynamic analysis tools in a real world environment. +## Beneficial Additions + +Providing the following items can bolster the efficiency of our review, allowing for a deeper and more thorough examination of your codebase. + +- [ ] **Ranges of all system parameters used in the system** – explicitly outline the minimum and maximum bound of configuration values in your codebase +- [ ] **Document all design decisions** – decision-making processes engineering tradeoffs, and discarded alternatives related to the system and codebase setup provides valuable context that greatly enhances our review efficiency, affording a more comprehensive and nuanced understanding of the entire system. +- [ ] **Provide results of fuzz and differential testing** – especially vital if your code involves lower-level assembly or math-related libraries. This aids us in employing tooling to spot deviations and edge cases. +- [ ] **Perform and report on stateful invariant testing** – such testing enables us to pinpoint edge cases that might occur during end-to-end operations. +- [ ] **Provide videos of complex workflows** – visual aids can effectively capture complex workflows, improving our grasp of the system's intricacies and bolstering the efficiency of our review. +- [ ] **Provide lists of actors with their expected roles and privileges** – outlining the distinct actors in your system, along with their specific roles and permissions, grants us a clearer understanding of how user interactions are designed and how authority is structured within the system. This, in turn, boosts the efficiency and depth of our review. +- [ ] **Prepare and share an Incident response plan** – a robust incident response plan provides insight into how your system is prepared to handle potential security breaches or unexpected system behavior. By sharing this, we can assess the system's robustness under adverse conditions, as well as provide recommendations for enhancing the resilience of your incident response strategy. + +### References + +- Our own experience +- [The Pragamatic Programmer](https://pragprog.com/titles/tpp20/the-pragmatic-programmer-20th-anniversary-edition/) \ No newline at end of file From ce8c2713797ef18e58280dad6252d47623ee9f39 Mon Sep 17 00:00:00 2001 From: Nat Chin Date: Thu, 7 Dec 2023 14:05:31 -0500 Subject: [PATCH 5/8] Run linter --- development-guidelines/review_checklist.md | 37 ++++++++++++---------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/development-guidelines/review_checklist.md b/development-guidelines/review_checklist.md index 44bab4de..8a88a2da 100644 --- a/development-guidelines/review_checklist.md +++ b/development-guidelines/review_checklist.md @@ -1,50 +1,53 @@ # Security Review Preparation Checklist -So, you're getting a security review! +So, you're getting a security review! Ensure the security review proceeds smoothly by taking prior action with the following informative items. -## Essential Items +## Essential Items + The items below guide us in focusing effectively on the pertinent parts of your code, maximizing our utilization of time and resources. - [ ] **Provide build instructions** – share how to build and test your code using a fresh clone of your repository. It gives us direct insight into your setup. -- [ ] **List exact files in scope for the review** – list out the exact files to be reviewed to focus on key areas of your codebase. -- [ ] **Freeze and share your hash/branch/release before the review starts** – a stable commit allows us to start digging into your codebase and learning how it works. Tell your security review team what commit to review, and try to ensure that this commit is frozen before the review commences. -- [ ] **Add inline comments** – utilise comments on complex areas of the codebase. Document why something was done, its purpose and goal. +- [ ] **List exact files in scope for the review** – list out the exact files to be reviewed to focus on key areas of your codebase. +- [ ] **Freeze and share your hash/branch/release before the review starts** – a stable commit allows us to start digging into your codebase and learning how it works. Tell your security review team what commit to review, and try to ensure that this commit is frozen before the review commences. +- [ ] **Add inline comments** – utilise comments on complex areas of the codebase. Document why something was done, its purpose and goal. - [ ] **Provide Natspec documentation** – ensure all functions have NatSpec descriptions for function's purpose, parameters, and return values. - [ ] **Provide test coverage report** – provide a report outlining tested and untested code areas. -- [ ] **Share your unit tests** – share your unit tests with us! We may use it to understand the system flow, can provide feedback on its setup, and can use it to test proof of concepts. +- [ ] **Share your unit tests** – share your unit tests with us! We may use it to understand the system flow, can provide feedback on its setup, and can use it to test proof of concepts. - [ ] **List external dependencies you're using and their purpose** – list used libraries and external dependencies along with their purposes. - [ ] **Create user flow diagrams** – outline the entrypoint functions, subsequent callstacks, and expected system interactions for users. - [ ] **Design an architecture diagram** – illustrate the interactions among contracts. - [ ] **Let us know what you're worried about** – reveal parts of the code or exploit paths that cause concern.This helps us target our review and make sure that we prioritize the areas of the codebase you're worried about. -- [ ] **Document the impossible** – know when you are making assumptions on types and their values into the codebase, and document all these instances. +- [ ] **Document the impossible** – know when you are making assumptions on types and their values into the codebase, and document all these instances. + +### Defi-Specific Additions -### Defi-Specific Additions - [ ] **Share economic analysis performed on the codebase** – sharing results of economic analysis will help us to understand the boundaries of inputs and ranges of outputs. - [ ] **List system invariants** – define your assumptions about system operations and function behaviors. -- [ ] **Provide documentation for arithmetic formulas used with code references** – all formulas you implement should be referenced in this document, with a link to the function in your code that implements it. If the representation in your code differs from the formula, provide the derivation that maps the formula used in the code and the formula you intend to implement. +- [ ] **Provide documentation for arithmetic formulas used with code references** – all formulas you implement should be referenced in this document, with a link to the function in your code that implements it. If the representation in your code differs from the formula, provide the derivation that maps the formula used in the code and the formula you intend to implement. - [ ] **Create a glossary for your system** – consistently used terminology should be documented and made accessible. -- [ ] **Validation of rounding directions** – share all analysis you have done on checking the correct rounding direction for your system. +- [ ] **Validation of rounding directions** – share all analysis you have done on checking the correct rounding direction for your system. ### Bridge-Specific Additions + - [ ] **Document off-chain checks** – provide details about validations performed by off-chain components like relayers or bots. -- [ ] **Checks performed by off-chain components such as relayers or bots** tell us what data validation takes place off-chain so we can better understand what data is sent to smart contracts -- [ ] **Transition of value-passing on source and destination chains** – show us how the chains share data, and what return values/emissions from a source chain maps to a destination chain +- [ ] **Checks performed by off-chain components such as relayers or bots** tell us what data validation takes place off-chain so we can better understand what data is sent to smart contracts +- [ ] **Transition of value-passing on source and destination chains** – show us how the chains share data, and what return values/emissions from a source chain maps to a destination chain ## Beneficial Additions Providing the following items can bolster the efficiency of our review, allowing for a deeper and more thorough examination of your codebase. -- [ ] **Ranges of all system parameters used in the system** – explicitly outline the minimum and maximum bound of configuration values in your codebase -- [ ] **Document all design decisions** – decision-making processes engineering tradeoffs, and discarded alternatives related to the system and codebase setup provides valuable context that greatly enhances our review efficiency, affording a more comprehensive and nuanced understanding of the entire system. +- [ ] **Ranges of all system parameters used in the system** – explicitly outline the minimum and maximum bound of configuration values in your codebase +- [ ] **Document all design decisions** – decision-making processes engineering tradeoffs, and discarded alternatives related to the system and codebase setup provides valuable context that greatly enhances our review efficiency, affording a more comprehensive and nuanced understanding of the entire system. - [ ] **Provide results of fuzz and differential testing** – especially vital if your code involves lower-level assembly or math-related libraries. This aids us in employing tooling to spot deviations and edge cases. - [ ] **Perform and report on stateful invariant testing** – such testing enables us to pinpoint edge cases that might occur during end-to-end operations. - [ ] **Provide videos of complex workflows** – visual aids can effectively capture complex workflows, improving our grasp of the system's intricacies and bolstering the efficiency of our review. - [ ] **Provide lists of actors with their expected roles and privileges** – outlining the distinct actors in your system, along with their specific roles and permissions, grants us a clearer understanding of how user interactions are designed and how authority is structured within the system. This, in turn, boosts the efficiency and depth of our review. - [ ] **Prepare and share an Incident response plan** – a robust incident response plan provides insight into how your system is prepared to handle potential security breaches or unexpected system behavior. By sharing this, we can assess the system's robustness under adverse conditions, as well as provide recommendations for enhancing the resilience of your incident response strategy. -### References +### References -- Our own experience -- [The Pragamatic Programmer](https://pragprog.com/titles/tpp20/the-pragmatic-programmer-20th-anniversary-edition/) \ No newline at end of file +- Our own experience +- [The Pragamatic Programmer](https://pragprog.com/titles/tpp20/the-pragmatic-programmer-20th-anniversary-edition/) From a094504111b67ec5dc675dd58edd5f7a3f9eddac Mon Sep 17 00:00:00 2001 From: Nat Chin Date: Thu, 7 Dec 2023 15:37:48 -0500 Subject: [PATCH 6/8] Made review checklist less text-based --- development-guidelines/review_checklist.md | 75 +++++++++++----------- 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/development-guidelines/review_checklist.md b/development-guidelines/review_checklist.md index 8a88a2da..187d1b86 100644 --- a/development-guidelines/review_checklist.md +++ b/development-guidelines/review_checklist.md @@ -1,53 +1,52 @@ # Security Review Preparation Checklist -So, you're getting a security review! +Get ready for your security review! Ensuring a few key elements are in place before the review starts can make the process significantly smoother for both sides. -Ensure the security review proceeds smoothly by taking prior action with the following informative items. +💡 **ProTip 1:** Predefine areas of focus and provide the review team early access to your codebase. -## Essential Items +- Provide a detailed list of files for review. +- Freeze a stable commit hash, branch, or release prior to review. +- Pinpoint areas in the codebase that previously had issues, inspire less confidence, or are of particular concern. +- If your codebase is a fork of an existing protocol, delinate the differences and modifications you made compared to the original codebase. -The items below guide us in focusing effectively on the pertinent parts of your code, maximizing our utilization of time and resources. +💡 **ProTip 2:** Lay the groundwork for your review by ensuring your project is build-ready. This allows us to focus on giving you actionable recommendations instead of trying to build your code! -- [ ] **Provide build instructions** – share how to build and test your code using a fresh clone of your repository. It gives us direct insight into your setup. -- [ ] **List exact files in scope for the review** – list out the exact files to be reviewed to focus on key areas of your codebase. -- [ ] **Freeze and share your hash/branch/release before the review starts** – a stable commit allows us to start digging into your codebase and learning how it works. Tell your security review team what commit to review, and try to ensure that this commit is frozen before the review commences. -- [ ] **Add inline comments** – utilise comments on complex areas of the codebase. Document why something was done, its purpose and goal. -- [ ] **Provide Natspec documentation** – ensure all functions have NatSpec descriptions for function's purpose, parameters, and return values. -- [ ] **Provide test coverage report** – provide a report outlining tested and untested code areas. -- [ ] **Share your unit tests** – share your unit tests with us! We may use it to understand the system flow, can provide feedback on its setup, and can use it to test proof of concepts. -- [ ] **List external dependencies you're using and their purpose** – list used libraries and external dependencies along with their purposes. -- [ ] **Create user flow diagrams** – outline the entrypoint functions, subsequent callstacks, and expected system interactions for users. -- [ ] **Design an architecture diagram** – illustrate the interactions among contracts. -- [ ] **Let us know what you're worried about** – reveal parts of the code or exploit paths that cause concern.This helps us target our review and make sure that we prioritize the areas of the codebase you're worried about. -- [ ] **Document the impossible** – know when you are making assumptions on types and their values into the codebase, and document all these instances. +- Create a clear set of build instructions. +- Confirm your setup process by cloning and testing your repository on a fresh environment. -### Defi-Specific Additions +💡 **ProTip 3:** Streamline our process of building a mental model of your codebase by providing comprehensive documentation. -- [ ] **Share economic analysis performed on the codebase** – sharing results of economic analysis will help us to understand the boundaries of inputs and ranges of outputs. -- [ ] **List system invariants** – define your assumptions about system operations and function behaviors. -- [ ] **Provide documentation for arithmetic formulas used with code references** – all formulas you implement should be referenced in this document, with a link to the function in your code that implements it. If the representation in your code differs from the formula, provide the derivation that maps the formula used in the code and the formula you intend to implement. -- [ ] **Create a glossary for your system** – consistently used terminology should be documented and made accessible. -- [ ] **Validation of rounding directions** – share all analysis you have done on checking the correct rounding direction for your system. +- Create flowcharts and sequence diagrams to depict primary workflows. +- List actors and with their respective roles and privileges. +- Incorporate external developer documentation that links directly to your code. +- Add inline comments for complex areas of your system. +- Maintain comprehensive NatSpec descriptions for all functions. +- Create short video walkthroughs for complex workflows or areas of concern. -### Bridge-Specific Additions +💡 **ProTip 4:** Share your test suite and coverage report with us to better understand the system. -- [ ] **Document off-chain checks** – provide details about validations performed by off-chain components like relayers or bots. -- [ ] **Checks performed by off-chain components such as relayers or bots** tell us what data validation takes place off-chain so we can better understand what data is sent to smart contracts -- [ ] **Transition of value-passing on source and destination chains** – show us how the chains share data, and what return values/emissions from a source chain maps to a destination chain +- Provide your test coverage report. +- Share unit and stateful fuzz tests. +- Share fuzz and differential tests. -## Beneficial Additions +💡 **ProTip 5:** For arithmetic-heavy codebases, meticulously document and map all of your formulas. -Providing the following items can bolster the efficiency of our review, allowing for a deeper and more thorough examination of your codebase. +- Document every formula implemented in your codebase. +- Map each formula to in-code implementation and if there are deviations between these two, include derivations. +- Share all rounding direction analysis. +- Share results from any economic analysis conducted on your codebase. -- [ ] **Ranges of all system parameters used in the system** – explicitly outline the minimum and maximum bound of configuration values in your codebase -- [ ] **Document all design decisions** – decision-making processes engineering tradeoffs, and discarded alternatives related to the system and codebase setup provides valuable context that greatly enhances our review efficiency, affording a more comprehensive and nuanced understanding of the entire system. -- [ ] **Provide results of fuzz and differential testing** – especially vital if your code involves lower-level assembly or math-related libraries. This aids us in employing tooling to spot deviations and edge cases. -- [ ] **Perform and report on stateful invariant testing** – such testing enables us to pinpoint edge cases that might occur during end-to-end operations. -- [ ] **Provide videos of complex workflows** – visual aids can effectively capture complex workflows, improving our grasp of the system's intricacies and bolstering the efficiency of our review. -- [ ] **Provide lists of actors with their expected roles and privileges** – outlining the distinct actors in your system, along with their specific roles and permissions, grants us a clearer understanding of how user interactions are designed and how authority is structured within the system. This, in turn, boosts the efficiency and depth of our review. -- [ ] **Prepare and share an Incident response plan** – a robust incident response plan provides insight into how your system is prepared to handle potential security breaches or unexpected system behavior. By sharing this, we can assess the system's robustness under adverse conditions, as well as provide recommendations for enhancing the resilience of your incident response strategy. +💡 **ProTip 6:** Expedite familiarisation of your codebase by detailing all assumptions. -### References +- List system invariants. +- Identify the parameter ranges (minimum and maximum values) used in your system. +- Highlight unreachable or logically excluded system states. +- Compile a glossary for consistent terminology use. +- List external dependencies used and their purpose. -- Our own experience -- [The Pragamatic Programmer](https://pragprog.com/titles/tpp20/the-pragmatic-programmer-20th-anniversary-edition/) +💡 **ProTip 7:** Clarify all interactions within the system. Highlight how contracts work together on-chain and how your contract interfaces with off-chain components. + +- Create an architecture diagram of on-chain contract interactions. +- Document all design decisions, including engineering trade-offs, and discarded alternatives. +- If your system uses off-chain components, outline data validation procedures off-chain and the input bounds for on-chain functions. +- If your system has bridge-like functionality, document values passing between source and destination chains. From 3ec99919859c0f80d59f979770911a13716e1b24 Mon Sep 17 00:00:00 2001 From: Feist Josselin Date: Wed, 21 Feb 2024 14:28:46 +0100 Subject: [PATCH 7/8] Improvements --- development-guidelines/review_checklist.md | 76 +++++++++++----------- 1 file changed, 39 insertions(+), 37 deletions(-) diff --git a/development-guidelines/review_checklist.md b/development-guidelines/review_checklist.md index 187d1b86..15307f07 100644 --- a/development-guidelines/review_checklist.md +++ b/development-guidelines/review_checklist.md @@ -1,52 +1,54 @@ -# Security Review Preparation Checklist +# How to prepare for a security review Get ready for your security review! Ensuring a few key elements are in place before the review starts can make the process significantly smoother for both sides. -💡 **ProTip 1:** Predefine areas of focus and provide the review team early access to your codebase. +## Set a goal for the review -- Provide a detailed list of files for review. -- Freeze a stable commit hash, branch, or release prior to review. -- Pinpoint areas in the codebase that previously had issues, inspire less confidence, or are of particular concern. -- If your codebase is a fork of an existing protocol, delinate the differences and modifications you made compared to the original codebase. +This is the most important step of a security review, and paradoxically the one most often overlooked. You should have an idea of what kind of questions you want answered, such as: -💡 **ProTip 2:** Lay the groundwork for your review by ensuring your project is build-ready. This allows us to focus on giving you actionable recommendations instead of trying to build your code! +- What’s the overall level of security for this product? +- What are the areas that you are the most concerns about? + - Take into considerations previous audits and issues, complex parts, and fragile components. +- What is the worst case scenario for your project? -- Create a clear set of build instructions. -- Confirm your setup process by cloning and testing your repository on a fresh environment. +Knowing your biggest area of concern will help the assessment team tailor their approach to meet your needs. -💡 **ProTip 3:** Streamline our process of building a mental model of your codebase by providing comprehensive documentation. +## Resolve the easy issues -- Create flowcharts and sequence diagrams to depict primary workflows. -- List actors and with their respective roles and privileges. -- Incorporate external developer documentation that links directly to your code. -- Add inline comments for complex areas of your system. -- Maintain comprehensive NatSpec descriptions for all functions. -- Create short video walkthroughs for complex workflows or areas of concern. +Handing the code off to the assessment team is a lot like releasing the product: the cleaner the code, the better everything will go. To that end: -💡 **ProTip 4:** Share your test suite and coverage report with us to better understand the system. +- **Triage all results from static analysis tools**. Go after the low-hanging fruits and use: + - [Slither](https://github.com/crytic/slither) for Solidity codebases + - [dylint](https://github.com/trailofbits/dylint) for Rust codebases + - [golangci](https://golangci-lint.run/) for Go codebases + - [CodeQL and Semgrep](https://appsec.guide/) for Go/Rust/C++/... codebases +- **Increase unit and feature test coverage**. Ideally this has been part of the development process, but everyone slips up, tests don’t get updated, or new features don’t quite match the old integrations tests. Now is the time to update the tests and run them all. +- **Remove dead code, unused libraries, and other extraneous weight.** You may know which is unused but the consultants won’t and will waste time investigating it for potential issues. The same goes for that new feature that hasn’t seen progress in months, or that third-party library that doesn’t get used anymore. -- Provide your test coverage report. -- Share unit and stateful fuzz tests. -- Share fuzz and differential tests. +## Ensure the code is accessible -💡 **ProTip 5:** For arithmetic-heavy codebases, meticulously document and map all of your formulas. +Making the code accessible and clearly identified will avoid wasting ressources from the security engineers. -- Document every formula implemented in your codebase. -- Map each formula to in-code implementation and if there are deviations between these two, include derivations. -- Share all rounding direction analysis. -- Share results from any economic analysis conducted on your codebase. +- **Provide a detailed list of files for review.**. This will avoid confusion if your codebase is large and some elements are not meant to be in scope. +- **Create a clear set of build instructions, and confirm the setup by cloning and testing your repository on a fresh environment.** A code that cannot be built is a code more difficult to review. +- **Freeze a stable commit hash, branch, or release prior to review.** Working on a moving target makes the review more difficult +- **Identify boilerplates, dependencies and difference from forked code**. By highliting what code you wrote, you will help keeping the review focused -💡 **ProTip 6:** Expedite familiarisation of your codebase by detailing all assumptions. +## Document, Document, Document -- List system invariants. -- Identify the parameter ranges (minimum and maximum values) used in your system. -- Highlight unreachable or logically excluded system states. -- Compile a glossary for consistent terminology use. -- List external dependencies used and their purpose. +Streamline the revuew process of building a mental model of your codebase by providing comprehensive documentation. -💡 **ProTip 7:** Clarify all interactions within the system. Highlight how contracts work together on-chain and how your contract interfaces with off-chain components. - -- Create an architecture diagram of on-chain contract interactions. -- Document all design decisions, including engineering trade-offs, and discarded alternatives. -- If your system uses off-chain components, outline data validation procedures off-chain and the input bounds for on-chain functions. -- If your system has bridge-like functionality, document values passing between source and destination chains. +- **Create flowcharts and sequence diagrams to depict primary workflows**. They will help identify the components and their relationships +- **Write users stories**. Having users stories is a powerful tool to explain a project +- **Outline the on-chain / off-chain assumptions**. This includes: + - Data validation procedure + - Oracles information + - Bridges assumptions +- **List actors and with their respective roles and privileges**. The complexity of a system grows with its number of actors. +- **Incorporate external developer documentation that links directly to your code**. This will help to ensure the documentation is up to date with the code +- **Add function documentation and inline comments for complex areas of your system**. Code documentation should include: + - System and function level invariants + - Parameter ranges (minimum and maximum values) used in your system. + - Arithmetic formula: how they map to their specification, and their precision loss exceptations +- **Compile a glossary for consistent terminology use.** You use your codebase every day and you are familar with the terminology - a new person looking at your code is not +- **Consider creating short video walkthroughs for complex workflows or areas of concern**. Video walkthroughs is a great format to share your knowledge From 6c69596fd6fc9ca8afc42f949b983d19cf27034e Mon Sep 17 00:00:00 2001 From: Feist Josselin Date: Wed, 21 Feb 2024 14:31:02 +0100 Subject: [PATCH 8/8] update summary --- SUMMARY.md | 1 + 1 file changed, 1 insertion(+) diff --git a/SUMMARY.md b/SUMMARY.md index b64f4537..e5f58417 100644 --- a/SUMMARY.md +++ b/SUMMARY.md @@ -8,6 +8,7 @@ - [Token Integration Checklist](./development-guidelines/token_integration.md) - [Incident Response Recommendations](./development-guidelines/incident_response.md) - [Secure Development Workflow](./development-guidelines/workflow.md) + - [Preparing for a Security Review](./development-guidelines/review_checklist.md) - [Learn EVM](./learn_evm/README.md) - [EVM Opcode Reference](./learn_evm/evm_opcodes.md) - [Transaction Tracing](./learn_evm/tracing.md)