diff --git a/not-so-smart-contracts/substrate/README.md b/not-so-smart-contracts/substrate/README.md new file mode 100644 index 00000000..9896200b --- /dev/null +++ b/not-so-smart-contracts/substrate/README.md @@ -0,0 +1,31 @@ +# (Not So) Smart Pallets + +This repository contains examples of common Substrate pallet vulnerabilities. Use Not So Smart Pallets to learn about Substrate vulnerabilities, as a reference when performing security reviews, and as a benchmark for security and analysis tools. + +## Features + +Each _Not So Smart Pallet_ includes a standard set of information: + +* Description of the vulnerability type +* Attack scenarios to exploit the vulnerability +* Recommendations to eliminate or mitigate the vulnerability +* A mock pallet that exhibits the flaw +* References to third-party resources with more information + +## Vulnerabilities + +| Not So Smart Pallet | Description | +| --- | --- | +| [Arithmetic overflow](./arithmetic_overflow/README.md) | Integer overflow due to incorrect use of arithmetic operators | +| [Don't panic!](./dont_panic/README.md) | System panics create a potential DoS attack vector | +| [Weights and fees](./weights_and_fees/README.md) | Incorrect weight calculations can create a potential DoS attack vector | +| [Verify first](./verify_first/README.md) | Verify first, write last | +| [Unsigned transaction validation](./validate_unsigned/README.md) | Insufficient validation of unsigned transactions | +| [Bad randomness](./randomness/README.md) | Unsafe sources of randomness in Substrate | +| [Bad origin](./origins/README.md) | Incorrect use of call origin can lead to bypassing access controls | + +## Credits + +These examples are developed and maintained by [Trail of Bits](https://www.trailofbits.com/). + +If you have questions, problems, or just want to learn more, then join the #ethereum channel on the [Empire Hacking Slack](https://empireslacking.herokuapp.com/) or [contact us](https://www.trailofbits.com/contact/) directly. \ No newline at end of file diff --git a/not-so-smart-contracts/substrate/arithmetic_overflow/README.md b/not-so-smart-contracts/substrate/arithmetic_overflow/README.md new file mode 100644 index 00000000..1d7adfb9 --- /dev/null +++ b/not-so-smart-contracts/substrate/arithmetic_overflow/README.md @@ -0,0 +1,44 @@ +# Arithmetic overflow + +Arithmetic overflow in Substrate occurs when arithmetic operations are performed using primitive operations instead of specialized functions that check for overflow. When a Substrate node is compiled in `debug` mode, integer overflows will cause the program to panic. However, when the node is compiled in `release` mode (e.g. `cargo build --release`), Substrate will perform two's complement wrapping. A production-ready node will be compiled in `release` mode, which makes it vulnerable to arithmetic overflow. + +# Example +In the [`pallet-overflow`](./pallet-overflow.rs) pallet, notice that the `transfer` function sets `update_sender` and `update_to` using primitive arithmetic operations. + + ```rust + /// Allow minting account to transfer a given balance to another account. + /// + /// Parameters: + /// - `to`: The account to receive the transfer. + /// - `amount`: The amount of balance to transfer. + /// + /// Emits `Transferred` event when successful. + #[pallet::weight(10_000)] + pub fn transfer( + origin: OriginFor, + to: T::AccountId, + amount: u64, + ) -> DispatchResultWithPostInfo { + let sender = ensure_signed(origin)?; + let sender_balance = Self::get_balance(&sender); + let receiver_balance = Self::get_balance(&to); + + // Calculate new balances. + let update_sender = sender_balance - amount; + let update_to = receiver_balance + amount; + [...] + } +``` + +The sender of the extrinsic can exploit this vulnerability by causing `update_sender` to underflow, which artificially inflates their balance. + +**Note**: Aside from the stronger mitigations mentioned below, a check to make sure that `sender` has at least `amount` balance would have also prevented an underflow. + +# Mitigations +- Use `checked` or `saturating` functions for arithmetic operations. + - [`CheckedAdd` trait](https://docs.rs/num/0.4.0/num/traits/trait.CheckedAdd.html) + - [`Saturating` trait](https://docs.rs/num/0.4.0/num/traits/trait.Saturating.html) + +# References +- https://doc.rust-lang.org/book/ch03-02-data-types.html#integer-overflow +- https://docs.substrate.io/reference/how-to-guides/basics/use-helper-functions/ diff --git a/not-so-smart-contracts/substrate/arithmetic_overflow/pallet-overflow.rs b/not-so-smart-contracts/substrate/arithmetic_overflow/pallet-overflow.rs new file mode 100644 index 00000000..87def723 --- /dev/null +++ b/not-so-smart-contracts/substrate/arithmetic_overflow/pallet-overflow.rs @@ -0,0 +1,113 @@ +#![cfg_attr(not(feature = "std"), no_std)] + +pub use pallet::*; +#[frame_support::pallet] +pub mod pallet { + use frame_support::{dispatch::DispatchResultWithPostInfo, pallet_prelude::*}; + use frame_system::pallet_prelude::*; + + /// Pallet configuration + #[pallet::config] + pub trait Config: frame_system::Config { + /// Because this pallet emits events, it depends on the runtime's definition of an event. + type Event: From> + IsType<::Event>; + } + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(_); + + // DEFAULT total supply of tokens + #[pallet::type_value] + pub(super) fn TotalSupplyDefaultValue() -> u64 { + 1000 + } + + // Data structure that holds the total supply of tokens + #[pallet::storage] + #[pallet::getter(fn total_supply)] + pub(super) type TotalSupply = + StorageValue<_, u64, ValueQuery, TotalSupplyDefaultValue>; + + // Data structure that holds whether or not the pallet's init() function has been called + #[pallet::storage] + #[pallet::getter(fn is_init)] + pub(super) type Init = StorageValue<_, bool, ValueQuery>; + + /// Storage item for balances to accounts mapping. + #[pallet::storage] + #[pallet::getter(fn get_balance)] + pub(super) type BalanceToAccount = StorageMap< + _, + Blake2_128Concat, + T::AccountId, + u64, + ValueQuery + >; + + /// Token mint can emit two Event types. + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event { + /// Token was initialized by user + Initialized(T::AccountId), + /// Tokens were successfully transferred between accounts. [from, to, value] + Transferred(T::AccountId, T::AccountId, u64), + } + + #[pallet::hooks] + impl Hooks> for Pallet {} + + #[pallet::error] + pub enum Error { + /// Attempted to initialize the token after it had already been initialized. + AlreadyInitialized, + } + + #[pallet::call] + impl Pallet { + /// Initialize the token + /// Transfers the total_supply amount to the caller + /// If init() has already been called, throw AlreadyInitialized error + #[pallet::weight(10_000)] + pub fn init(origin: OriginFor) -> DispatchResultWithPostInfo { + let sender = ensure_signed(origin)?; + ensure!(!Self::is_init(), >::AlreadyInitialized); + + >::insert(sender, Self::total_supply()); + + Init::::put(true); + Ok(().into()) + } + + /// Allow minting account to transfer a given balance to another account. + /// + /// Parameters: + /// - `to`: The account to receive the transfer. + /// - `amount`: The amount of balance to transfer. + /// + /// Emits `Transferred` event when successful. + #[pallet::weight(10_000)] + pub fn transfer( + origin: OriginFor, + to: T::AccountId, + amount: u64, + ) -> DispatchResultWithPostInfo { + let sender = ensure_signed(origin)?; + let sender_balance = Self::get_balance(&sender); + let receiver_balance = Self::get_balance(&to); + + // Calculate new balances. + let update_sender = sender_balance - amount; + let update_to = receiver_balance + amount; + + // Update both accounts storage. + >::insert(&sender, update_sender); + >::insert(&to, update_to); + + // Emit event. + Self::deposit_event(Event::Transferred(sender, to, amount)); + Ok(().into()) + } + } +} \ No newline at end of file diff --git a/not-so-smart-contracts/substrate/dont_panic/README.md b/not-so-smart-contracts/substrate/dont_panic/README.md new file mode 100644 index 00000000..45a36467 --- /dev/null +++ b/not-so-smart-contracts/substrate/dont_panic/README.md @@ -0,0 +1,39 @@ +# Don't Panic! + +Panics occur when the node enters a state that it cannot handle and stops the program / process instead of trying to proceed. Panics can occur for a large variety of reasons such as out-of-bounds array access, incorrect data validation, type conversions, and much more. A well-designed Substrate node must NEVER panic! If a node panics, it opens up the possibility for a denial-of-service (DoS) attack. + +# Example +In the [`pallet-dont-panic`](./pallet-dont-panic.rs) pallet, the `find_important_value` dispatchable checks to see if `useful_amounts[0]` is greater than `1_000`. If so, it sets the `ImportantVal` `StorageValue` to the value held in `useful_amounts[0]`. + +```rust + /// Do some work + /// + /// Parameters: + /// - `useful_amounts`: A vector of u64 values in which there is a important value. + /// + /// Emits `FoundVal` event when successful. + #[pallet::weight(10_000)] + pub fn find_important_value( + origin: OriginFor, + useful_amounts: Vec, + ) -> DispatchResultWithPostInfo { + let sender = ensure_signed(origin)?; + + ensure!(useful_amounts[0] > 1_000, >::NoImportantValueFound); + + // Found the important value + ImportantValue::::put(&useful_amounts[0]); + [...] + } +``` + +However, notice that there is no check before the array indexing to see whether the length of `useful_amounts` is greater than zero. Thus, if `useful_amounts` is empty, the indexing will cause an array out-of-bounds error which will make the node panic. Since the `find_important_value` function is callable by anyone, an attacker can set `useful_amounts` to an empty array and spam the network with malicious transactions to launch a DoS attack. + +# Mitigations +- Write non-throwing Rust code (e.g. prefer returning [`Result`](https://paritytech.github.io/substrate/master/frame_support/dispatch/result/enum.Result.html) types, use [`ensure!`](https://paritytech.github.io/substrate/master/frame_support/macro.ensure.html), etc.). +- Proper data validation of all input parameters is crucial to ensure that an unexpected panic does not occur. +- A thorough suite of unit tests should be implemented. +- Fuzz testing (e.g. using [`test-fuzz`](https://github.com/trailofbits/test-fuzz)) can aid in exploring more of the input space. + +# References +- https://docs.substrate.io/main-docs/build/events-errors/#errors diff --git a/not-so-smart-contracts/substrate/dont_panic/pallet-dont-panic.rs b/not-so-smart-contracts/substrate/dont_panic/pallet-dont-panic.rs new file mode 100644 index 00000000..caf87898 --- /dev/null +++ b/not-so-smart-contracts/substrate/dont_panic/pallet-dont-panic.rs @@ -0,0 +1,68 @@ +#![cfg_attr(not(feature = "std"), no_std)] + +pub use pallet::*; + +#[frame_support::pallet] +pub mod pallet { + use frame_support::{dispatch::DispatchResultWithPostInfo, pallet_prelude::*}; + use frame_system::pallet_prelude::*; + use sp_std::prelude::*; + + /// Pallet configuration + #[pallet::config] + pub trait Config: frame_system::Config { + /// Because this pallet emits events, it depends on the runtime's definition of an event. + type Event: From> + IsType<::Event>; + } + + #[pallet::pallet] + #[pallet::without_storage_info] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(_); + + /// Storage item for holding an ImportantValue + #[pallet::storage] + #[pallet::getter(fn get_val)] + pub(super) type ImportantValue = StorageValue<_, u64, ValueQuery>; + + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event { + /// Emit after val is found + FoundVal(T::AccountId, u64), + } + + #[pallet::hooks] + impl Hooks> for Pallet {} + + #[pallet::error] + pub enum Error { + NoImportantValueFound, + } + + #[pallet::call] + impl Pallet { + /// Find important value + /// + /// Parameters: + /// - `useful_amounts`: A vector of u64 values in which there is a important value. + /// + /// Emits `FoundVal` event when successful. + #[pallet::weight(10_000)] + pub fn find_important_value( + origin: OriginFor, + useful_amounts: Vec, + ) -> DispatchResultWithPostInfo { + let sender = ensure_signed(origin)?; + + ensure!(useful_amounts[0] > 1_000, >::NoImportantValueFound); + + // Found the important value + ImportantValue::::put(&useful_amounts[0]); + + // Emit event + Self::deposit_event(Event::FoundVal(sender, useful_amounts[0])); + Ok(().into()) + } + } +} \ No newline at end of file diff --git a/not-so-smart-contracts/substrate/origins/README.md b/not-so-smart-contracts/substrate/origins/README.md new file mode 100644 index 00000000..111177d8 --- /dev/null +++ b/not-so-smart-contracts/substrate/origins/README.md @@ -0,0 +1,59 @@ +# Origins + +The origin of a call tells a dispatchable function where the call has come from. Origins are a way to implement access controls in the system. + +There are three types of origins that can used in the runtime: + +```rust +pub enum RawOrigin { + Root, + Signed(AccountId), + None, +} +``` + +Outside of the out-of-box origins, custom origins can also be created that are catered to a specific runtime. The primary use case for custom origins is to configure privileged access to dispatch calls in the runtime, outside of `RawOrigin::Root`. + +Using privileged origins, like `RawOrigin::Root` or custom origins, can lead to access control violations if not used correctly. It is a common error to use `ensure_signed` in place of `ensure_root` which would allow any user to bypass the access control placed by using `ensure_root`. + +# Example +In the [`pallet-bad-origin`](./pallet-bad-origin.rs) pallet, there is a `set_important_val` function that should be only callable by the `ForceOrigin` _custom_ origin type. This custom origin allows the pallet to specify that only a specific account can call `set_important_val`. + +```rust +#[pallet::call] +impl Pallet { + /// Set the important val + /// Should be only callable by ForceOrigin + #[pallet::weight(10_000)] + pub fn set_important_val( + origin: OriginFor, + new_val: u64 + ) -> DispatchResultWithPostInfo { + let sender = ensure_signed(origin)?; + // Change to new value + >::put(new_val); + + // Emit event + Self::deposit_event(Event::ImportantValSet(sender, new_val)); + + Ok(().into()) + } +} +``` +However, the `set_important_val` is using `ensure_signed`; this allows any account to set `ImportantVal`. To allow only the `ForceOrigin` to call `set_important_val` the following change can be made: + +```rust +T::ForceOrigin::ensure_origin(origin.clone())?; +let sender = ensure_signed(origin)?; +``` + +# Mitigations +- Ensure that the correct access controls are placed on privileged functions. +- Develop user documentation on all risks associated with the system, including those associated with privileged users. +- A thorough suite of unit tests that validates access controls is crucial. + +# References +- https://docs.substrate.io/main-docs/build/origins/ +- https://docs.substrate.io/tutorials/work-with-pallets/specify-the-origin-for-a-call/ +- https://paritytech.github.io/substrate/master/pallet_sudo/index.html# +- https://paritytech.github.io/substrate/master/pallet_democracy/index.html diff --git a/not-so-smart-contracts/substrate/origins/pallet-bad-origin.rs b/not-so-smart-contracts/substrate/origins/pallet-bad-origin.rs new file mode 100644 index 00000000..1b84bc4d --- /dev/null +++ b/not-so-smart-contracts/substrate/origins/pallet-bad-origin.rs @@ -0,0 +1,61 @@ +#![cfg_attr(not(feature = "std"), no_std)] + +pub use pallet::*; + +#[frame_support::pallet] +pub mod pallet { + use frame_support::{dispatch::DispatchResultWithPostInfo, pallet_prelude::{*, ValueQuery}}; + use frame_system::pallet_prelude::*; + use sp_std::prelude::*; + + /// Pallet configuration + #[pallet::config] + pub trait Config: frame_system::Config { + /// Because this pallet emits events, it depends on the runtime's definition of an event. + type Event: From> + IsType<::Event>; + + // Specifies the account that can perform some action + type ForceOrigin: EnsureOrigin; + } + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(_); + + /// Storage item for important value that should be editable only by root + #[pallet::storage] + #[pallet::getter(fn get_important_val)] + pub(super) type ImportantVal = StorageValue<_, u64, ValueQuery>; + + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event { + /// Emit when new important val is set. + ImportantValSet(T::AccountId, u64), + } + + #[pallet::hooks] + impl Hooks> for Pallet {} + + #[pallet::call] + impl Pallet { + /// Set the important val + /// Should be only callable by ForceOrigin + #[pallet::weight(10_000)] + pub fn set_important_val( + origin: OriginFor, + new_val: u64 + ) -> DispatchResultWithPostInfo { + T::ForceOrigin::ensure_origin(origin.clone())?; + let sender = ensure_signed(origin)?; + // Change to new value + >::put(new_val); + + // Emit event + Self::deposit_event(Event::ImportantValSet(sender, new_val)); + + Ok(().into()) + } + } + +} \ No newline at end of file diff --git a/not-so-smart-contracts/substrate/randomness/README.md b/not-so-smart-contracts/substrate/randomness/README.md new file mode 100644 index 00000000..9d932a88 --- /dev/null +++ b/not-so-smart-contracts/substrate/randomness/README.md @@ -0,0 +1,61 @@ +# Bad Randomness + +To use randomness in a Substrate pallet, all you need to do is require a source of randomness in the `Config` trait of a pallet. This source of Randomness must implement the [`Randomness`](https://paritytech.github.io/substrate/master/frame_support/traits/trait.Randomness.html) trait. The trait provides two methods for obtaining randomness. +1. `random_seed`: This function takes no arguments and returns back a random value. Calling this value multiple times in a block will result in the same value. +2. `random`: Takes in a byte-array (a.k.a "context-identifier") and returns a value that is as independent as possible from other contexts. + +Substrate provides the [Randomness Collective Flip Pallet](https://paritytech.github.io/substrate/master/pallet_randomness_collective_flip/index.html) and a Verifiable Random Function implementation in the [BABE pallet](https://paritytech.github.io/substrate/master/pallet_babe/index.html). Developers can also choose to build their own source of randomness. + +A bad source of randomness can lead to a variety of exploits such as the theft of funds or undefined system behavior. + +# Example +The [`pallet-bad-lottery`](./pallet-bad-lottery.rs) pallet is a simplified "lottery" system that requires one to guess the next random number. If they guess correctly, they are the winner of the lottery. + +```rust +#[pallet::call] +impl Pallet { +/// Guess the random value +/// If you guess correctly, you become the winner +#[pallet::weight(10_000)] +pub fn guess( + origin: OriginFor, + guess: T::Hash +) -> DispatchResultWithPostInfo { + let sender = ensure_signed(origin)?; + // Random value. + let nonce = Self::get_and_increment_nonce(); + let (random_value, _) = T::MyRandomness::random(&nonce); + // Check if guess is correct + ensure!(guess == random_value, >::IncorrectGuess); + >::put(&sender); + + Self::deposit_event(Event::NewWinner(sender)); + + Ok(().into()) +} +} + +impl Pallet { +/// Increment the nonce each time guess() is called +pub fn get_and_increment_nonce() -> Vec { + let nonce = Nonce::::get(); + Nonce::::put(nonce.wrapping_add(1)); + nonce.encode() +} +} +``` + +Note that the quality of randomness provided to the `pallet-bad-lottery` pallet is related to the randomness source. If the randomness source is the "Randomness Collective Flip Pallet", this lottery system is insecure. This is because the collective flip pallet implements "low-influence randomness". This makes it vulnerable to a collusion attack where a small minority of participants can give the same random number contribution making it highly likely to have the seed be this random number (click [here](https://ethresear.ch/t/rng-exploitability-analysis-assuming-pure-randao-based-main-chain/1825/7) to learn more). Additionally, as mentioned in the Substrate documentation, "low-influence randomness can be useful when defending against relatively weak adversaries. Using this pallet as a randomness source is advisable primarily in low-security situations like testing." + +# Mitigations +- Use the randomness implementation provided by the [BABE pallet](https://paritytech.github.io/substrate/master/pallet_babe/index.html). This pallet provides "production-grade randomness, and is used in Polkadot. **Selecting this randomness source dictates that your blockchain use Babe consensus.**" +- Defer from creating a custom source of randomness unless specifically necessary for the runtime being developed. +- Do not use `random_seed` as the method of choice for randomness unless specifically necessary for the runtime being developed. + + +# References +- https://docs.substrate.io/main-docs/build/randomness/ +- https://docs.substrate.io/reference/how-to-guides/pallet-design/incorporate-randomness/ +- https://ethresear.ch/t/rng-exploitability-analysis-assuming-pure-randao-based-main-chain/1825/7 +- https://ethresear.ch/t/collective-coin-flipping-csprng/3252/21 +- https://github.com/paritytech/ink/issues/57#issuecomment-486998848 \ No newline at end of file diff --git a/not-so-smart-contracts/substrate/randomness/pallet-bad-lottery.rs b/not-so-smart-contracts/substrate/randomness/pallet-bad-lottery.rs new file mode 100644 index 00000000..3db15eee --- /dev/null +++ b/not-so-smart-contracts/substrate/randomness/pallet-bad-lottery.rs @@ -0,0 +1,82 @@ +#![cfg_attr(not(feature = "std"), no_std)] + +pub use pallet::*; + +#[frame_support::pallet] +pub mod pallet { + use frame_support::{dispatch::DispatchResultWithPostInfo, pallet_prelude::{*, ValueQuery}}; + use frame_system::pallet_prelude::*; + use frame_support::traits::Randomness; + use sp_std::prelude::*; + + /// Pallet configuration + #[pallet::config] + pub trait Config: frame_system::Config { + /// Because this pallet emits events, it depends on the runtime's definition of an event. + type Event: From> + IsType<::Event>; + /// Create a randomness type for this pallet + type MyRandomness: Randomness; + } + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(_); + + /// Storage item for nonce + #[pallet::storage] + #[pallet::getter(fn get_nonce)] + pub(super) type Nonce = StorageValue<_, u64, ValueQuery>; + + /// Storage item for current winner + #[pallet::storage] + #[pallet::getter(fn get_winner)] + pub(super) type Winner = StorageValue<_, T::AccountId, OptionQuery>; + + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event { + /// Emit when new winner is found + NewWinner(T::AccountId), + } + + #[pallet::hooks] + impl Hooks> for Pallet {} + + #[pallet::error] + pub enum Error { + /// Guessed the wrong number. + IncorrectGuess, + } + + #[pallet::call] + impl Pallet { + /// Guess the random value + /// If you guess correctly, you become the winner + #[pallet::weight(10_000)] + pub fn guess( + origin: OriginFor, + guess: T::Hash + ) -> DispatchResultWithPostInfo { + let sender = ensure_signed(origin)?; + // Random value. + let nonce = Self::get_and_increment_nonce(); + let (random_value, _) = T::MyRandomness::random(&nonce); + // Check if guess is correct + ensure!(guess == random_value, >::IncorrectGuess); + >::put(&sender); + + Self::deposit_event(Event::NewWinner(sender)); + + Ok(().into()) + } + } + + impl Pallet { + /// Increment the nonce each time guess() is called + pub fn get_and_increment_nonce() -> Vec { + let nonce = Nonce::::get(); + Nonce::::put(nonce.wrapping_add(1)); + nonce.encode() + } + } +} \ No newline at end of file diff --git a/not-so-smart-contracts/substrate/validate_unsigned/README.md b/not-so-smart-contracts/substrate/validate_unsigned/README.md new file mode 100644 index 00000000..d1a85e5b --- /dev/null +++ b/not-so-smart-contracts/substrate/validate_unsigned/README.md @@ -0,0 +1,49 @@ +# Unsigned Transaction Validation + +There are three types of transactions allowed in a Substrate runtime: signed, unsigned, and inherent. An unsigned transaction does not require a signature and does not include information about who sent the transaction. This is naturally problematic because there is no by-default deterrent to spam or replay attacks. Because of this, Substrate allows users to create custom functions to validate unsigned transaction. However, incorrect or improper validation of an unsigned transaction can allow _anyone_ to perform potentially malicious actions. Usually, unsigned transactions are allowed only for select parties (e.g., off-chain workers (OCW)). But, if improper data validation of an unsigned transaction allows a malicious actor to spoof data as if it came from an OCW, this can lead to unexpected behavior. Additionally, improper validation opens up the possibility to replay attacks where the same transaction can be sent to the transaction pool again and again to perform some malicious action repeatedly. + +The validation of an unsigned transaction must be provided by the pallet that chooses to accept them. To allow unsigned transactions, a pallet must implement the [`frame_support::unsigned::ValidateUnsigned`](https://paritytech.github.io/substrate/master/frame_support/attr.pallet.html#validate-unsigned-palletvalidate_unsigned-optional) trait. The `validate_unsigned` function, which must be implemented as part of the `ValidateUnsigned` trait, will provide the logic necessary to ensure that the transaction is valid. An off chain worker (OCW) can be implemented directly in a pallet using the `offchain_worker` [hook](https://paritytech.github.io/substrate/master/frame_support/attr.pallet.html#hooks-pallethooks-optional). The OCW can send an unsigned transaction by calling [`SubmitTransaction::submit_unsigned_transaction`](https://paritytech.github.io/substrate/master/frame_system/offchain/struct.SubmitTransaction.html). Upon submission, the `validate_unsigned` function will ensure that the transaction is valid and then pass the transaction on towards towards the final dispatchable function. + +# Example + +The [`pallet-bad-unsigned`](./pallet-bad-unsigned.rs) pallet is an example that showcases improper unsigned transaction validation. The pallet tracks the average, rolling price of some "asset"; this price data is being retrieved by an OCW. The `fetch_price` function, which is called by the OCW, naively returns 100 as the current price (note that an [HTTP request](https://github.com/paritytech/substrate/blob/e8a7d161f39db70cb27fdad6c6e215cf493ebc3b/frame/examples/offchain-worker/src/lib.rs#L572-L625) can be made here for true price data). The `validate_unsigned` function (see below) simply validates that the `Call` is being made to `submit_price_unsigned` and nothing else. +```rust +/// By default unsigned transactions are disallowed, but implementing the validator +/// here we make sure that some particular calls (the ones produced by offchain worker) +/// are being whitelisted and marked as valid. +fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity { + // If `submit_price_unsigned` is being called, the transaction is valid. + // Otherwise, it is an InvalidTransaction. + if let Call::submit_price_unsigned { block_number, price: new_price } = call { + let avg_price = Self::average_price() + .map(|price| if &price > new_price { price - new_price } else { new_price - price }) + .unwrap_or(0); + + let valid_tx = | provide | { + ValidTransaction::with_tag_prefix("ExampleOffchainWorker") + .priority(T::UnsignedPriority::get().saturating_add(avg_price as _)) + .and_provides([&provide]) + .longevity(5) + .propagate(true) + .build() + }; + valid_tx(b"submit_price_unsigned".to_vec()) + } else { + InvalidTransaction::Call.into() + } +} +``` + +However, notice that there is nothing preventing an attacker from sending malicious price data. Both `block_number` and `price` can be set to arbitrary values. For `block_number`, it would be valuable to ensure that it is not a block number in the future; only price data for the current block can be submitted. Additionally, medianization can be used to ensure that the reported price is not severely affected by outliers. Finally, unsigned submissions can be throttled by enforcing a delay after each submission. + +Note that the simplest solution would be to sign the offchain submissions so that the runtime can validate that a known OCW is sending the price submission transactions. + +# Mitigations +- Consider whether unsigned transactions is a requirement for the runtime that is being built. OCWs can also submit signed transactions or transactions with signed payloads. +- Ensure that each parameter provided to `validate_unsigned` is validated to prevent the runtime from entering a state that is vulnerable or undefined. +# References +- https://docs.substrate.io/main-docs/fundamentals/transaction-types/#unsigned-transactions +- https://docs.substrate.io/main-docs/fundamentals/offchain-operations/ +- https://github.com/paritytech/substrate/blob/polkadot-v0.9.26/frame/examples/offchain-worker/src/lib.rs +- https://docs.substrate.io/reference/how-to-guides/offchain-workers/offchain-transactions/ +- https://docs.substrate.io/reference/how-to-guides/offchain-workers/offchain-http-requests/ \ No newline at end of file diff --git a/not-so-smart-contracts/substrate/validate_unsigned/pallet-bad-unsigned.rs b/not-so-smart-contracts/substrate/validate_unsigned/pallet-bad-unsigned.rs new file mode 100644 index 00000000..167086ab --- /dev/null +++ b/not-so-smart-contracts/substrate/validate_unsigned/pallet-bad-unsigned.rs @@ -0,0 +1,186 @@ +/// Source: https://github.com/paritytech/substrate/blob/e8a7d161f39db70cb27fdad6c6e215cf493ebc3b/frame/examples/offchain-worker/src/lib.rs +/// This example was re-purposed from the above source +#![cfg_attr(not(feature = "std"), no_std)] + +use frame_support::traits::Get; +use frame_system::{ + self as system, + offchain::{SubmitTransaction, SendTransactionTypes}, +}; +use sp_runtime::{ + transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction}, +}; + +pub use pallet::*; + +#[frame_support::pallet] +pub mod pallet { + use super::*; + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + + /// This pallet's configuration trait + #[pallet::config] + pub trait Config: SendTransactionTypes> + frame_system::Config { + + /// The overarching event type. + type Event: From> + IsType<::Event>; + + /// The overarching dispatch call type. + type Call: From>; + + /// Maximum number of prices. + #[pallet::constant] + type MaxPrices: Get; + + /// A configuration for base priority of unsigned transactions. + /// + /// This is exposed so that it can be tuned for particular runtime, when + /// multiple pallets send unsigned transactions. + #[pallet::constant] + type UnsignedPriority: Get; + } + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(_); + + /// A vector of recently submitted prices. + /// This is used to calculate average price, should have bounded size. + #[pallet::storage] + #[pallet::getter(fn prices)] + pub(super) type Prices = StorageValue<_, BoundedVec, ValueQuery>; + + /// Events for the pallet. + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event { + /// Event generated when new price is accepted to contribute to the list of prices. + NewPrice { price: u32, block_number: T::BlockNumber, maybe_who: Option }, + } + + #[pallet::hooks] + impl Hooks> for Pallet { + /// Offchain Worker entry point. + /// By implementing `fn offchain_worker` you declare a new offchain worker. + fn offchain_worker(block_number: T::BlockNumber) { + log::info!("Hello I'm an offchain worker!"); + + // Print debug information + let parent_hash = >::block_hash(block_number - 1u32.into()); + log::debug!("Current block: {:?} (parent hash: {:?}", block_number, parent_hash); + + // Get and log average price + let average: Option = Self::average_price(); + log::debug!("Current price: {:?}", average); + + // Send unsigned transaction to log latest price from an offchain resource + let res = Self::fetch_price_and_send_raw_unsigned(block_number); + if let Err(e) = res { + // Throw error + log::error!("Error: {}", e); + } + } + } + + /// A public part of the pallet. + #[pallet::call] + impl Pallet { + /// Submit new price to the list via unsigned transaction. + /// + /// It's important to specify `weight` for unsigned calls as well, because even though + /// they don't charge fees, we still don't want a single block to contain unlimited + /// number of such transactions. + #[pallet::weight(0)] + pub fn submit_price_unsigned( + origin: OriginFor, + _block_number: T::BlockNumber, + price: u32, + ) -> DispatchResultWithPostInfo { + // This ensures that the function can only be called via unsigned transaction. + ensure_none(origin)?; + // Add the price to the on-chain list, but mark it as coming from an empty address. + Self::add_price(None, price, _block_number); + Ok(().into()) + } + + } + + /// Validate unsigned call to this module. + #[pallet::validate_unsigned] + impl ValidateUnsigned for Pallet { + type Call = Call; + + /// By default unsigned transactions are disallowed, but implementing the validator + /// here we make sure that some particular calls (the ones produced by offchain worker) + /// are being whitelisted and marked as valid. + fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity { + // If `submit_price_unsigned` is being called, the transaction is valid. + // Otherwise, it is an InvalidTransaction. + if let Call::submit_price_unsigned { block_number, price: new_price } = call { + let avg_price = Self::average_price() + .map(|price| if &price > new_price { price - new_price } else { new_price - price }) + .unwrap_or(0); + + let valid_tx = | provide | { + ValidTransaction::with_tag_prefix("ExampleOffchainWorker") + .priority(T::UnsignedPriority::get().saturating_add(avg_price as _)) + .and_provides([&provide]) + .longevity(5) + .propagate(true) + .build() + }; + valid_tx(b"submit_price_unsigned".to_vec()) + } else { + InvalidTransaction::Call.into() + } + } + } +} + +// Helper functions of the pallet +impl Pallet { + + /// A helper function to fetch the price and send a raw unsigned transaction. + fn fetch_price_and_send_raw_unsigned(block_number: T::BlockNumber) -> Result<(), &'static str> { + // Make an external "HTTP request" to fetch the current price. + // Note this call will block until response is received. + let price = Self::fetch_price(); + + // Create wrapped `Call` struct that will end up calling `submit_price_unsigned` + let call = Call::submit_price_unsigned { block_number, price }; + + // Submit the unsigned transaction on-chain + SubmitTransaction::>::submit_unsigned_transaction(call.into()) + .map_err(|()| "Unable to submit unsigned transaction.")?; + + Ok(()) + } + + + /// Naively "fetch" current price and return the result. + fn fetch_price() -> u32 { + 1000 + } + + /// Calculate current average price. + fn average_price() -> Option { + let prices = >::get(); + if prices.is_empty() { + None + } else { + Some(prices.iter().fold(0_u32, |a, b| a.saturating_add(*b)) / prices.len() as u32) + } + } + + /// Add new price to the list. + fn add_price(maybe_who: Option, price: u32, block_number: T::BlockNumber) { + >::mutate(|prices| { + if prices.try_push(price).is_err() { + prices[(price % T::MaxPrices::get()) as usize] = price; + } + }); + // Here we are raising the NewPrice event + Self::deposit_event(Event::NewPrice { price, block_number, maybe_who }); + } +} \ No newline at end of file diff --git a/not-so-smart-contracts/substrate/verify_first/README.md b/not-so-smart-contracts/substrate/verify_first/README.md new file mode 100644 index 00000000..c8e373da --- /dev/null +++ b/not-so-smart-contracts/substrate/verify_first/README.md @@ -0,0 +1,54 @@ +# Verify First, Write Last + +**NOTE**: As of [Polkadot v0.9.25](https://github.com/substrate-developer-hub/substrate-docs/issues/1215), the **Verify First, Write Last** practice is no longer required. However, since older versions are still vulnerable and because it is still best practice, it is worth discussing the "Verify First, Write Last" idiom. + +Substrate does not cache state prior to extrinsic dispatch. Instead, state changes are made as they are invoked. This is in contrast to a transaction in Ethereum where, if the transaction reverts, no state changes will persist. In the case of Substrate, if a state change is made and then the dispatch throws a `DispatchError`, the original state change will persist. This unique behavior has led to the "Verify First, Write Last" practice. + +```rust +{ + // all checks and throwing code go here + + // ** no throwing code below this line ** + + // all event emissions & storage writes go here +} +``` +# Example +In the [`pallet-verify-first`](./pallet-verify-first.rs) pallet, the `init` dispatchable is used to set up the `TotalSupply` of the token and transfer them to the `sender`. `init` should be only called once. Thus, the `Init` boolean is set to `true` when it is called initially. If `init` is called more than once, the transaction will throw an error because `Init` is already `true`. + +```rust +/// Initialize the token +/// Transfers the total_supply amount to the caller +/// If init() has already been called, throw AlreadyInitialized error +#[pallet::weight(10_000)] +pub fn init( + origin: OriginFor, + supply: u64 +) -> DispatchResultWithPostInfo { + let sender = ensure_signed(origin)?; + + if supply > 0 { + >::put(&supply); + } + // Set sender's balance to total_supply() + >::insert(&sender, supply); + + // Revert above changes if init() has already been called + ensure!(!Self::is_init(), >::AlreadyInitialized); + + // Set Init StorageValue to `true` + Init::::put(true); + + // Emit event + Self::deposit_event(Event::Initialized(sender)); + + Ok(().into()) +} +``` + +However, notice that the setting of `TotalSupply` and the transfer of funds happens before the check on `Init`. This violates the "Verify First, Write Last" practice. In an older version of Substrate, this would allow a malicious `sender` to call `init` multiple times and change the value of `TotalSupply` and their balance of the token. + +# Mitigations +- Follow the "Verify First, Write Last" practice by doing all the necessary data validation before performing state changes and emitting events. +# References +- https://docs.substrate.io/main-docs/build/runtime-storage/#best-practices \ No newline at end of file diff --git a/not-so-smart-contracts/substrate/verify_first/pallet-verify-first.rs b/not-so-smart-contracts/substrate/verify_first/pallet-verify-first.rs new file mode 100644 index 00000000..9c558654 --- /dev/null +++ b/not-so-smart-contracts/substrate/verify_first/pallet-verify-first.rs @@ -0,0 +1,130 @@ +#![cfg_attr(not(feature = "std"), no_std)] + +pub use pallet::*; + +#[frame_support::pallet] +pub mod pallet { + use frame_support::{dispatch::DispatchResultWithPostInfo, pallet_prelude::*}; + use frame_system::pallet_prelude::*; + + /// Pallet configuration + #[pallet::config] + pub trait Config: frame_system::Config { + /// Because this pallet emits events, it depends on the runtime's definition of an event. + type Event: From> + IsType<::Event>; + } + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(_); + + // DEFAULT Total supply of tokens + #[pallet::type_value] + pub(super) fn TotalSupplyDefaultValue() -> u64 { + 1000 + } + + // Data structure that holds the total supply of tokens + #[pallet::storage] + #[pallet::getter(fn total_supply)] + pub(super) type TotalSupply = + StorageValue<_, u64, ValueQuery, TotalSupplyDefaultValue>; + + // Data structure that holds whether or not the pallet's init() function has been called + #[pallet::storage] + #[pallet::getter(fn is_init)] + pub(super) type Init = StorageValue<_, bool, ValueQuery>; + + /// Storage item for balances to accounts mapping. + #[pallet::storage] + #[pallet::getter(fn get_balance)] + pub(super) type BalanceToAccount = StorageMap< + _, + Blake2_128Concat, + T::AccountId, + u64, + ValueQuery + >; + + /// Token mint can emit two Event types. + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event { + /// Token was initialized by user + Initialized(T::AccountId), + /// Tokens were successfully transferred between accounts. [from, to, value] + Transferred(T::AccountId, T::AccountId, u64), + } + + #[pallet::hooks] + impl Hooks> for Pallet {} + + #[pallet::error] + pub enum Error { + /// Attempted to initialize the token after it had already been initialized. + AlreadyInitialized, + /// Attempted to transfer more funds than were available + InsufficientFunds, + } + + #[pallet::call] + impl Pallet { + /// Initialize the token + /// Transfers the total_supply amount to the caller + /// If init() has already been called, throw AlreadyInitialized error + #[pallet::weight(10_000)] + pub fn init( + origin: OriginFor, + supply: u64 + ) -> DispatchResultWithPostInfo { + let sender = ensure_signed(origin)?; + + if supply > 0 { + >::put(&supply); + } + // Set sender's balance to total_supply() + >::insert(&sender, supply); + + // Revert above changes if init() has already been called + ensure!(!Self::is_init(), >::AlreadyInitialized); + + // Set Init StorageValue to `true` + Init::::put(true); + + // Emit event + Self::deposit_event(Event::Initialized(sender)); + + Ok(().into()) + } + + /// Allow minting account to transfer a given balance to another account. + /// + /// Parameters: + /// - `to`: The account to receive the transfer. + /// - `amount`: The amount of balance to transfer. + /// + /// Emits `Transferred` event when successful. + #[pallet::weight(10_000)] + pub fn transfer( + origin: OriginFor, + to: T::AccountId, + amount: u64, + ) -> DispatchResultWithPostInfo { + let sender = ensure_signed(origin)?; + let sender_balance = Self::get_balance(&sender); + let receiver_balance = Self::get_balance(&to); + + // Calculate new balances. + let update_sender = sender_balance.checked_sub(amount).ok_or(>::InsufficientFunds)?; + let update_to = receiver_balance.checked_add(amount).expect("Entire supply should fit in u64"); + + // Update both accounts storage. + >::insert(&sender, update_sender); + >::insert(&to, update_to); + + // Emit event. + Self::deposit_event(Event::Transferred(sender, to, amount)); + Ok(().into()) + } + } +} \ No newline at end of file diff --git a/not-so-smart-contracts/substrate/weights_and_fees/README.md b/not-so-smart-contracts/substrate/weights_and_fees/README.md new file mode 100644 index 00000000..41741fd4 --- /dev/null +++ b/not-so-smart-contracts/substrate/weights_and_fees/README.md @@ -0,0 +1,49 @@ +# Weights and Fees + +Weights and transaction fees are the two main ways to regulate the consumption of blockchain resources. The overuse of blockchain resources can allow a malicious actor to spam the network to cause a denial-of-service (DoS). Weights are used to manage the time it takes to validate the block. The larger the weight, the more "resources" / time the computation takes. Transaction fees provide an economic incentive to limit the number of resources used to perform operations; the fee for a given transaction is a function of the weight required by the transaction. + +Weights can be fixed or a custom "weight annotation / function" can be implemented. A weight function can calculate the weight, for example, based on the number of database read / writes and the size of the input paramaters (e.g. a long `Vec<>`). To optimize the weight such that users do not pay too little or too much for a transaction, benchmarking can be used to empirically determine the correct weight in worst case scenarios. + +Specifying the correct weight function and benchmarking it is crucial to protect the Substrate node from denial-of-service (DoS) attacks. Since fees are a function of weight, a bad weight function implies incorrect fees. For example, if some function performs heavy computation (which takes a lot of time) but specifies a very small weight, it is cheap to call that function. In this way an attacker can perform a low-cost attack while still stealing a large amount of block execution time. This will prevent regular transactions from being fit into those blocks. + +# Example +In the [`pallet-bad-weights`](./pallet-bad-weights.rs) pallet, a custom weight function, `MyWeightFunction`, is used to calculate the weight for a call to `do_work`. The weight required for a call to `do_work` is `10_000_000` times the length of the `useful_amounts` vector. + +```rust +impl WeighData<(&Vec,)> for MyWeightFunction { + fn weigh_data(&self, (amounts,): (&Vec,)) -> Weight { + self.0.saturating_mul(amounts.len() as u64).into() + } +} +``` + +However, if the length of the `useful_amounts` vector is zero, the weight to call `do_work` would be zero. A weight of zero implies that calling this function is financially cheap. This opens the opportunity for an attacker to call `do_work` a large number of times to saturate the network with malicious transactions without having to pay a large fee and could cause a DoS attack. + +One potential fix for this is to set a fixed weight if the length of `useful_amounts` is zero. + +```rust +impl WeighData<(&Vec,)> for MyWeightFunction { + fn weigh_data(&self, (amounts,): (&Vec,)) -> Weight { + // The weight function is `y = mx + b` where `m` and `b` are both `self.0` (the static fee) and `x` is the length of the `amounts` array. + // If `amounts.len() == 0` then the weight is simply the static fee (i.e. `y = b`) + self.0 + self.0.saturating_mul(amounts.len() as u64).into() + } +} +``` + +In the example above, if the length of `amounts` (i.e. `useful_amounts`) is zero, then the function will return `self.0` (i.e. `10_000_000`). + +On the other hand, if an attacker sends a `useful_amounts` vector that is incredibly large, the returned `Weight` can become large enough such that the dispatchable takes up a large amount block execution time and prevents other transactions from being fit into the block. A fix for this would be to bound the maximum allowable length for `useful_amounts`. + +**Note**: Custom _fee_ functions can also be created. These functions should also be carefully evaluated and tested to ensure that the risk of DoS attacks is mitigated. +# Mitigations +- Use [benchmarking](https://docs.substrate.io/main-docs/test/benchmark/) to empirically test the computational resources utilized by various dispatchable functions. Additionally, use benchmarking to define a lower and upper weight bound for each dispatchable. +- Create bounds for input arguments to prevent a transaction from taking up too many computational resources. For example, if a `Vec<>` is being taken as an input argument to a function, prevent the length of the `Vec<>` from being too large. +- Be wary of fixed weight dispatchables (e.g. `#[pallet::weight(1_000_000)]`). A weight that is completely agnostic to database read / writes or input parameters may open up avenues for DoS attacks. + +# References +- https://docs.substrate.io/main-docs/build/tx-weights-fees/ +- https://docs.substrate.io/reference/how-to-guides/weights/add-benchmarks/ +- https://docs.substrate.io/reference/how-to-guides/weights/use-custom-weights/ +- https://docs.substrate.io/reference/how-to-guides/weights/use-conditional-weights/ +- https://www.shawntabrizi.com/substrate/substrate-weight-and-fees/ \ No newline at end of file diff --git a/not-so-smart-contracts/substrate/weights_and_fees/pallet-bad-weights.rs b/not-so-smart-contracts/substrate/weights_and_fees/pallet-bad-weights.rs new file mode 100644 index 00000000..02860807 --- /dev/null +++ b/not-so-smart-contracts/substrate/weights_and_fees/pallet-bad-weights.rs @@ -0,0 +1,85 @@ +#![cfg_attr(not(feature = "std"), no_std)] + +pub use pallet::*; + +#[frame_support::pallet] +pub mod pallet { + use frame_support::{dispatch::DispatchResultWithPostInfo, pallet_prelude::*, weights::*}; + use frame_system::pallet_prelude::*; + use sp_std::prelude::*; + + /// Pallet configuration + #[pallet::config] + pub trait Config: frame_system::Config { + /// Because this pallet emits events, it depends on the runtime's definition of an event. + type Event: From> + IsType<::Event>; + } + + #[pallet::pallet] + #[pallet::without_storage_info] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(_); + + /// Storage item for useful_amounts passed to the do_work function + #[pallet::storage] + #[pallet::getter(fn useful_amounts)] + pub(super) type UsefulAmounts = StorageValue<_, Vec, ValueQuery>; + + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event { + /// Emit after do_work successfully completes + DidWork(T::AccountId), + } + + #[pallet::hooks] + impl Hooks> for Pallet {} + + /// Custom weight function implementation + pub struct MyWeightFunction(u64); + + /// The weight is linearly proportional to the length of the amounts array + impl WeighData<(&Vec,)> for MyWeightFunction { + fn weigh_data(&self, (amounts,): (&Vec,)) -> Weight { + self.0.saturating_mul(amounts.len() as u64).into() + } + } + + /// Custom weight function implementations need to implement the PaysFee trait + impl PaysFee for MyWeightFunction { + fn pays_fee(&self, _: T) -> Pays { + Pays::Yes + } + } + + /// Custom weight function implementations need to implement the ClassifyDispatch trait + impl ClassifyDispatch for MyWeightFunction { + fn classify_dispatch(&self, _: T) -> DispatchClass { + // Classify all calls as Normal (which is the default) + Default::default() + } + } + + #[pallet::call] + impl Pallet { + /// Do some work + /// + /// Parameters: + /// - `useful_amount`: A vector of u64 values that we want to store. + /// + /// Emits `DidWork` event when successful. + #[pallet::weight(MyWeightFunction(10_000_000))] + pub fn do_work( + origin: OriginFor, + useful_amounts: Vec, + ) -> DispatchResultWithPostInfo { + let sender = ensure_signed(origin)?; + UsefulAmounts::::put(useful_amounts); + // Do other important constant-time (O(1)) work + + // Emit event + Self::deposit_event(Event::DidWork(sender)); + Ok(().into()) + } + } +} \ No newline at end of file diff --git a/program-analysis/echidna/end-to-end-testing.md b/program-analysis/echidna/end-to-end-testing.md index c398e13c..866703c5 100644 --- a/program-analysis/echidna/end-to-end-testing.md +++ b/program-analysis/echidna/end-to-end-testing.md @@ -24,18 +24,20 @@ $ cd drizzle-box $ npm i truffle ``` -If ganache and ganache-cli are not installed, add them manually. In our example, we will run: +If `ganache` is not installed, add it manually. In our example, we will run: ``` -$ npm i ganache ganache-cli +$ npm i ganache ``` Other projects using yarn will require: ``` -$ yarn add ganache ganache-cli +$ yarn add ganache ``` +Ensure that `$ ganache --version` outputs `ganache v7.3.2` or greater. + It is also important to select *one* test script from the available tests. Ideally, this test will deploy all (or most) contracts, including mock/test ones. For this example, we are going to take a look to the `SimpleStorage` contract: ```solidity @@ -78,11 +80,31 @@ Before starting to write interesting properties, it is necessary to to collect a First, start Etheno: +```bash +$ etheno --ganache --ganache-args="--miner.blockGasLimit 10000000" -x init.json ``` -$ etheno --ganache --ganache-args "--deterministic --gasLimit 10000000" -x init.json + +By default the following Ganache arguments are set via Etheno: +- `-d`: Ganache will use a pre-defined, deterministic seed to create all accounts. +- `--chain.allowUnlimitedContractSize`: Allows unlimited contract sizes while debugging. This is set so that there is no size limitations on the contracts that are going to be deployed +- `-p `: The `port_num` will be set to (1) the value of `--ganache-port` or (2) Etheno will choose the smallest port number higher than the port number on which Etheno’s JSON RPC server is running. + +**NOTE:** If you are using Docker to run etheno, the commands should be: + +```bash +$ docker run -it -p 8545:8545 -v ~/etheno:/home/etheno/ trailofbits/etheno +(you will now be working within the Docker instance) +$ etheno --ganache --ganache-args="--miner.blockGasLimit 10000000" -x init.json ``` +- The `-p` in the _first command_ publishes (i.e. exposes) port 8545 from inside the Docker container out to port 8545 on the host. +- The `-v` in the _first command_ maps a directory from inside the Docker container to one outside the Docker container. After Etheno exits, the `init.json` file will now be in the `~/etheno` folder on the host. + -If Etheno fails to produce any output, then it fails to execute `ganache`. Check if `ganache` can be executed correctly from your terminal. + +Note that if the deployment fails to complete successfully due to a `ProviderError: exceeds block gas limit` exception, increasing the `--miner.blockGasLimit` value can help. +This is especially helpful for large contract deployments. Learn more about the various Ganache command-line arguments that can be set by clicking [here](https://www.npmjs.com/package/ganache). + +Additionally, if Etheno fails to produce any output, then it failed to execute `ganache` under-the-hood. Check if `ganache` (with the associated command-line arguments) can be executed correctly from your terminal without the use of Etheno. Meanwhile, in another terminal, run *one* test or the deployment process. How to run it depends on how the project was developed. For instance, for truffle, use: @@ -102,9 +124,9 @@ In the Drizzle example, we will run: $ truffle test test/simplestorage.js --network develop. ``` -After Etheno finishes, gently kill it using ctrl+c (twice). It will save the `init.json` file. If your test fails for some reason or you want to run a different one, restart etheno and re-run the test. +After Etheno finishes, gently kill it by using Ctrl+c (or Command+C on Mac). It will save the `init.json` file. If your test fails for some reason or you want to run a different one, restart Etheno and re-run the test. -## Writing and running a property: +## Writing and running a property Once we have a json file with saved transactions, we can verify that the `SimpleStorage` contract is deployed at `0x871DD7C2B4b25E1Aa18728e9D5f2Af4C4e431f5c`, so we can easily write a contract (`./contracts/crytic/E2E.sol`) with a simple a property to test it: @@ -119,6 +141,8 @@ contract E2E { } ``` +For large, multi-contract deployments, using `console.log` to print out the deployed, contract addresses can be valuable in quickly setting up the Echidna testing contract. + This simple property checks if the stored data remains constant. To run it you will need the following echidna config file (`echidna.yaml`): ```yaml @@ -140,4 +164,9 @@ crytic_const_storage: failed!💥 For this last step, make sure you are using `.` as a target for `echidna-test`. If you use the path to the `E2E.sol` file instead, then Echidna will not be able get information from all the deployed contracts to call the `set(uint256)` function and the property will never fail. +## Key considerations: +When using Etheno with Echidna, note that there are two edge cases that may cause unexpected behavior: +1. Function calls that use ether: The accounts that are created and used for testing in Ganache are not the same as the accounts that are used to send transactions in Echidna. Thus, the account balances of the Ganache accounts do not carry over to the accounts used by Echidna. So, if there is a function call logged by Etheno that requires the transfer of some ether from an account that exists in Ganache, this call will fail in Echidna. +2. Fuzz tests that rely on `block.timestamp`: The concept of time is different between Ganache and Echidna. Echidna always starts with a fixed timestamp while Etheno will use Ganache's concept of time. This means that assertions or requirements in a fuzz test that rely on timestamp comparisons / evaluations may fail in Echidna. + In the next part of this tutorial, we will explore how to easily find where contracts are deployed with a specific tool based on Slither. This will be useful if the deployment process is complex and we need to test an specific contract.