diff --git a/README.md b/README.md index d8d37eef..9e7a2743 100644 --- a/README.md +++ b/README.md @@ -51,10 +51,10 @@ Visit [Scout's website](https://coinfabrik.github.io/scout/) to view the full do | ------------------------------------------------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- | | [integer-overflow-or-underflow](https://coinfabrik.github.io/scout/docs/detectors/integer-overflow-or-underflow) | [An arithmetic operation overflows or underflows the available memory allocated to the variable.](https://coinfabrik.github.io/scout/docs/vulnerabilities/integer-overflow-or-underflow) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-1), [2](https://github.com/CoinFabrik/scout/tree/main/test-cases/integer-overflow-or-underflow/integer-overflow-or-underflow-2) | Critical | | [set-contract-storage](https://coinfabrik.github.io/scout/docs/detectors/set-contract-storage) | [Insufficient access control on set_contract_storage() function.](https://coinfabrik.github.io/scout/docs/vulnerabilities/set-contract-storage) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/set-contract-storage/set-contract-storage-1) | Critical | -| [reentrancy](https://coinfabrik.github.io/scout/docs/detectors/reentrancy) | [Consistency of contract state under recursive calls.](https://coinfabrik.github.io/scout/docs/vulnerabilities/reentrancy) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy/reentrancy-1), [2](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy/reentrancy-2) | Critical | -| [panic-error](https://coinfabrik.github.io/scout/docs/detectors/panic-error.md) | [Code panics on error instead of using descriptive enum.](https://coinfabrik.github.io/scout/docs/vulnerabilities/panic-error.md) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/panic-error/panic-error-1) | Enhancement | +| [reentrancy](https://coinfabrik.github.io/scout/docs/detectors/reentrancy) | [Consistency of contract state under recursive calls.](https://coinfabrik.github.io/scout/docs/vulnerabilities/reentrancy) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy-1/reentrancy-1), [2](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy-1/reentrancy-2), [3](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy-2/reentrancy-1) | Critical | +| [panic-error](https://coinfabrik.github.io/scout/docs/detectors/panic-error) | [Code panics on error instead of using descriptive enum.](https://coinfabrik.github.io/scout/docs/vulnerabilities/panic-error) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/panic-error/panic-error-1) | Enhancement | | [unused-return-enum](https://coinfabrik.github.io/scout/docs/detectors/unused-return-enum) | [Return enum from a function is not completely used.](https://coinfabrik.github.io/scout/docs/vulnerabilities/unused-return-enum) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/unused-return-enum/unused-return-enum-1) | Minor | -| [dos-unbounded-operation](https://coinfabrik.github.io/scout/docs/detectors/dos-unbounded-operation) | [DoS due to unbounded operation.](https://github.com/CoinFabrik/scout/blob/main/docs/docs/vulnerabilities/6-dos-unbounded-operation) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/dos-unbounded-operation/dos-unbounded-operation-1) | Medium | +| [dos-unbounded-operation](https://coinfabrik.github.io/scout/docs/detectors/dos-unbounded-operation) | [DoS due to unbounded operation.](https://coinfabrik.github.io/scout/docs/vulnerabilities/dos-unbounded-operation) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/dos-unbounded-operation/dos-unbounded-operation-1) | Medium | | [dos-unexpected-revert-with-vector](https://coinfabrik.github.io/scout/docs/detectors/dos-unexpected-revert-with-vector) | [DoS due to improper storage.](https://coinfabrik.github.io/scout/docs/vulnerabilities/dos-unexpected-revert-with-vector) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/dos-unexpected-revert-with-vector/dos-unexpected-revert-with-vector-1) | Medium | | [unsafe-expect](https://coinfabrik.github.io/scout/docs/detectors/unsafe-expect) | [Improper usage of the expect method, leading to unexpected program crashes.](https://coinfabrik.github.io/scout/docs/vulnerabilities/unsafe-expect) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/unsafe-expect/unsafe-expect-1) | Medium | | [unsafe-unwrap](https://coinfabrik.github.io/scout/docs/detectors/unsafe-unwrap) | [Inappropriate usage of the unwrap method, causing unexpected program crashes.](https://coinfabrik.github.io/scout/docs/vulnerabilities/unsafe-unwrap) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/unsafe-unwrap/unsafe-unwrap-1) | Medium | @@ -62,16 +62,17 @@ Visit [Scout's website](https://coinfabrik.github.io/scout/) to view the full do | [delegate-call](https://coinfabrik.github.io/scout/docs/detectors/delegate-call) | [Invoking code in another contract keeping the first contract's context.](https://coinfabrik.github.io/scout/docs/vulnerabilities/delegate-call) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/delegate-call/delegate-call-1) | Critical | | [zero-or-test-address](https://coinfabrik.github.io/scout/docs/detectors/zero-or-test-address) | [Avoid zero or test address assignment to prevent contract control loss.](https://coinfabrik.github.io/scout/docs/vulnerabilities/zero-or-test-address) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/zero-or-test-address/zero-or-test-address-1) | Medium | | [insufficiently-random-values](https://coinfabrik.github.io/scout/docs/detectors/insufficiently-random-values) | [Avoid using block attributes for random number generation to prevent manipulation.](https://coinfabrik.github.io/scout/docs/vulnerabilities/insufficiently-random-values) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/insufficiently-random-values/insufficiently-random-values-1) | Critical | -| [unrestricted-transfer-from](https://coinfabrik.github.io/scout/docs/detectors/unrestricted-transfer-from) | [Avoid passing an user-defined parameter as a `from` field in transfer-from ](https://coinfabrik.github.io/scout/docs/vulnerabilities/unrestricted-transfer-from) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/unrestricted-transfer-from/unrestricted-transfer-from-1) | High | +| [unrestricted-transfer-from](https://coinfabrik.github.io/scout/docs/detectors/unrestricted-transfer-from) | [Avoid passing an user-defined parameter as a `from` field in transfer-from ](https://coinfabrik.github.io/scout/docs/vulnerabilities/unrestricted-transfer-from) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/unrestricted-transfer-from/unrestricted-transfer-from-1) | Critical | | [assert-violation](https://coinfabrik.github.io/scout/docs/detectors/assert-violation) | [Avoid the usage of the macro `assert!`, it can panic.](https://coinfabrik.github.io/scout/docs/vulnerabilities/assert-violation) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/assert-violation/assert-violation-1) | Enhacement | -| [avoid-core-mem-forget](https://github.com/CoinFabrik/scout/tree/main/detectors/avoid-core-mem-forget) | [The use of core::mem::forget could lead to memory leaks and logic errors](https://coinfabrik.github.io/scout/docs/vulnerabilities/avoid-core-mem-forget) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1) | Enhacement | -| [avoid-format!-string](https://github.com/CoinFabrik/scout/tree/main/detectors/avoid-format!-string) | [The `format!` macro is not recommended. A custom error is recommended instead.](https://coinfabrik.github.io/scout/docs/vulnerabilities/avoid-format!-string) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/avoid-format!-string/avoid-format!-string-1) | Enhacement | -| [unprotected-self-destruct](https://github.com/CoinFabrik/scout/tree/main/detectors/unprotected-self-destruct) | [If users are allowed to call terminate_contract, they can intentionally or accidentally destroy the contract.](https://coinfabrik.github.io/scout/docs/vulnerabilities/unprotected-self-destruct) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/unprotected-self-destruct/unprotected-self-destruct-1) | Critical | -| [iterators-over-indexing](https://github.com/CoinFabrik/scout/tree/main/detectors/avoid-format!-string) | [Iterating with hardcoded indexes is slower than using an iterator. Also, if the index is out of bounds, it will panic.](https://coinfabrik.github.io/scout/docs/vulnerabilities/iterators-over-indexing) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/iterators-over-indexing/iterators-over-indexing-1) | Enhacement | -| [ink-version](https://github.com/CoinFabrik/scout/tree/main/detectors/ink-version) | [Using a pinned version of ink! can be dangerous, as it may have bugs or security issues. Use the latest version available.](https://coinfabrik.github.io/scout/docs/vulnerabilities/ink-version) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/ink-version/ink-version-1) | Enhacement | -| [unprotected-set-code-hash](https://github.com/CoinFabrik/scout/tree/main/detectors/set-code-hash) | [If users are allowed to call terminate_contract, they can intentionally modify the contract behaviour.](https://coinfabrik.github.io/scout/docs/vulnerabilities/unprotected-set-code-hash) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/set-code-hash/set-code-hash-1) | Critical | -| [unprotected-mapping-operation](https://github.com/CoinFabrik/scout/tree/main/detectors/unprotected-mapping-operation) | [Modifying mappings with an arbitrary key given by the user could lead to unintented modifications of critical data, modifying data belonging to other users, causing denial of service, unathorized access, and other potential issues.](https://coinfabrik.github.io/scout/docs/vulnerabilities/unprotected-mapping-operation) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1) | Critical | -| [lazy-delegate](https://github.com/CoinFabrik/scout/tree/main/detectors/lazy-delegate) | [Delegated calls in ink! need lazy storage.](https://coinfabrik.github.io/scout/docs/vulnerabilities/lazy-delegate) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/lazy-delegate/lazy-delegate-1) | Critical | +| [avoid-core-mem-forget](https://coinfabrik.github.io/scout/docs/detectors/avoid-core-mem-forget) | [The use of core::mem::forget could lead to memory leaks and logic errors](https://coinfabrik.github.io/scout/docs/vulnerabilities/avoid-core-mem-forget) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1) | Enhacement | +| [avoid-format-string](https://coinfabrik.github.io/scout/docs/detectors/avoid-format-string) | [The `format!` macro is not recommended. A custom error is recommended instead.](https://coinfabrik.github.io/scout/docs/vulnerabilities/avoid-format-string) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/avoid-format-string/avoid-format-string-1) | Enhacement | +| [unprotected-self-destruct](https://coinfabrik.github.io/scout/docs/detectors/unprotected-self-destruct) | [If users are allowed to call terminate_contract, they can intentionally or accidentally destroy the contract.](https://coinfabrik.github.io/scout/docs/vulnerabilities/unprotected-self-destruct) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/unprotected-self-destruct/unprotected-self-destruct-1) | Critical | +| [iterators-over-indexing](https://coinfabrik.github.io/scout/docs/detectors/iterators-over-indexing) | [Iterating with hardcoded indexes is slower than using an iterator. Also, if the index is out of bounds, it will panic.](https://coinfabrik.github.io/scout/docs/vulnerabilities/iterators-over-indexing) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/iterators-over-indexing/iterators-over-indexing-1) | Enhacement | +| [ink-version](https://coinfabrik.github.io/scout/docs/detectors/ink-version) | [Using a pinned version of ink! can be dangerous, as it may have bugs or security issues. Use the latest version available.](https://coinfabrik.github.io/scout/docs/vulnerabilities/ink-version) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/ink-version/ink-version-1) | Enhacement | +| [unprotected-set-code-hash](https://coinfabrik.github.io/scout/docs/detectors/unprotected-set-code-hash) | [If users are allowed to call terminate_contract, they can intentionally modify the contract behaviour.](https://coinfabrik.github.io/scout/docs/vulnerabilities/unprotected-set-code-hash) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/set-code-hash/set-code-hash-1) | Critical | +| [unprotected-mapping-operation](https://coinfabrik.github.io/scout/docs/detectors/unprotected-mapping-operation) | [Modifying mappings with an arbitrary key given by the user could lead to unintented modifications of critical data, modifying data belonging to other users, causing denial of service, unathorized access, and other potential issues.](https://coinfabrik.github.io/scout/docs/vulnerabilities/unprotected-mapping-operation) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1) | Critical | +| [lazy-delegate](https://coinfabrik.github.io/scout/docs/detectors/lazy-delegate) | [Delegated calls in ink! need lazy storage.](https://coinfabrik.github.io/scout/docs/vulnerabilities/lazy-delegate) | [1](https://github.com/CoinFabrik/scout/tree/main/test-cases/lazy-delegate/lazy-delegate-1) | Critical | + ## Tests diff --git a/docs/docs/detectors/3-reentrancy.md b/docs/docs/detectors/3-reentrancy.md index 4fc1605e..2a2c7799 100644 --- a/docs/docs/detectors/3-reentrancy.md +++ b/docs/docs/detectors/3-reentrancy.md @@ -72,4 +72,4 @@ if amount <= caller_balance { ### Implementation -The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout/tree/main/detectors/reentrancy). +The detector's implementation can be found at these links [link1](https://github.com/CoinFabrik/scout/tree/main/detectors/reentrancy-1), [link2](https://github.com/CoinFabrik/scout/tree/main/detectors/reentrancy-2). diff --git a/docs/docs/vulnerabilities/3-reentrancy.md b/docs/docs/vulnerabilities/3-reentrancy.md index ab1e2d86..2d18bf4d 100644 --- a/docs/docs/vulnerabilities/3-reentrancy.md +++ b/docs/docs/vulnerabilities/3-reentrancy.md @@ -3,7 +3,7 @@ * Vulnerability Category: `Reentrancy` * Severity: `Critical` * Detectors: [`reentrancy-1`](https://github.com/CoinFabrik/scout/tree/main/detectors/reentrancy-1), [`reentrancy-2`](https://github.com/CoinFabrik/scout/tree/main/detectors/reentrancy-2) -* Test Cases: [`reentrancy-1`](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy/reentrancy-1), [`reentrancy-2`](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy/reentrancy-2), [`reentrancy-3`](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy/reentrancy-3) +* Test Cases: [`reentrancy-1`](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy-1/reentrancy-1), [`reentrancy-2`](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy-1/reentrancy-2), [`reentrancy-3`](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy-2/reentrancy-1) Smart contracts can call other contracts and send tokens to them. These operations imply external calls where control flow is passed to the called @@ -110,12 +110,12 @@ pub fn exploit(&mut self) { ``` -The vulnerable code example can be found [here](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy/reentrancy-1/vulnerable-example). +The vulnerable code example can be found [here](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy-1/reentrancy-1/vulnerable-example). ### Deployment Vault and Exploit files can be found under the directories -[vulnerable-example/exploit](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy/reentrancy-1/vulnerable-example/exploit) and -[vulnerable-example/vault](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy/reentrancy-1/vulnerable-example/vault). +[vulnerable-example/exploit](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy-1/reentrancy-1/vulnerable-example/exploit) and +[vulnerable-example/vault](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy-1/reentrancy-1/vulnerable-example/vault). The whole exploit example can be run automatically using the `deploy.sh` file. ## Recommendation @@ -123,7 +123,7 @@ In general, risks associated to reentrancy can be addressed with the Check-Effect-Interaction pattern, a best practice that indicates that external calls should be the last thing to be executed in a function. In this example, this can be done by inserting the balance before transferring the value (see -[remediated-example-1](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy/reentrancy-1/remediated-example)). +[remediated-example-1](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy-1/reentrancy-1/remediated-example)). ```rust @@ -156,11 +156,11 @@ pub fn call_with_value(&mut self, address: AccountId, amount: Balance, selector: } ``` -The remediated code example can be found [here](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy/reentrancy-1/remediated-example). +The remediated code example can be found [here](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy-1/reentrancy-1/remediated-example). Alternatively, if reentrancy by an external contract is not needed, the `set_allow_reentry(true)` should be removed altogether (see -[remediated-example-2](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy/reentrancy-2/remediated-example)). This is equivalent in Substrate to using a +[remediated-example-2](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy-1/reentrancy-2/remediated-example)). This is equivalent in Substrate to using a [reentrancy guard](https://github.com/Supercolony-net/openbrush-contracts/tree/main/contracts/src/security/reentrancy_guard) like the one offered by [OpenBrush](https://github.com/Supercolony-net/openbrush-contracts). @@ -191,7 +191,7 @@ pub fn call_with_value(&mut self, address: AccountId, amount: Balance, selector: } ``` -The remediated code example can be found [here](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy/reentrancy-2/remediated-example). +The remediated code example can be found [here](https://github.com/CoinFabrik/scout/tree/main/test-cases/reentrancy-1/reentrancy-2/remediated-example). ## References * https://use.ink/datastructures/storage-layout