-
Notifications
You must be signed in to change notification settings - Fork 142
CosmWasm security best practices
The commandments:
- Don't deserialize into
Addr
. - Build systems that can be upgraded.
- Design for failure.
Before we get into those, I'd like to provide a prelude about our threat model:
DAO DAO DAOs manage millions of dollars. Every time you commit a line of code to our smart contracts you should feel comfortable wagering that money on it working.
I do not know of an environment more hostile to software than blockchains. From a security perspective they are a hellscape of instant finality, immutable code, and pseudonymity. On top of that, our code is open-source, and all of our APIs are callable by anyone with a private key. Our only saving graces are determinism and atomicity, though even that works against us when contracts lock.
This is not to say that we can not write safe code. It is to say: we are in a uniquely challenging environment, we must approach our work with the utmost care and diligence.
CosmWasm's Addr
type is not validated during deserialization. This means that messages with the Addr
type have unexpected deserialization properties. For example, the following JSON payload will successfully deserialize into the corresponding enum:
{ "set_address": { "addr": "🏴☠️" } }
pub enum ExecuteMsg {
SetAddress { addr: Addr }
}
To avoid unvalidated addresses in state, the String
type should be used and then validated.
pub enum ExecuteMsg {
SetAddress { addr: String }
}
// In the message handler.
let addr = deps.api.addr_validate(&addr).traspose()?;
Note the usage of map
and transpose
above. Where clarity is not impacted, we prefer standard library functions to custom logic on types. The Rust Standard Library is an extremely well used, security and performance conscious piece of software.
We are not clairvoyant. The systems we design should reflect this and be upgradable.
In order of most to least invasive, there are three ways a smart contract can be upgraded:
- A CosmWasm migration
- A config change
- A parent module swaps out a child module
When designing a system, choose the smallest hammer to meet your needs.
As an example of a module upgrade: say we're building a cw721-send rate limiter. Contract's that would like to use the rate limiter implement a proxy interface so they may receive the proxied NFTs, and can swap out and disable their proxy.
How do we allow users of this rate limiter to change the rate limit? We could add a contract-level admin, messages for nominating and confirming a new admin, and messages for updating the config.
To avoid this complexity, we may instead encourage consumers of our rate limited NFTs to create and use a new rate limiter. Now, to update the rate limit a consumer just removes the current rate limiter and installs a new one. This makes our rate limiting contract immutable without sacrificing functionality.
We are fallible. We should build systems that expect modules to fail and can detect and recover from those failures.
For example:
- Receiver of proposal module hooks are removed of they fail handling a message to stop the parent module from locking.
- Our proposal deposit system allows anyone to create proposals if a panic occurs during deposit refund logic.
- A bridge may allow withdrawals of NFTs if it detects its counterparty on another chain has failed.
When writing tests, write tests that test functionality and then write more that test contract behavior under duress. Example.
There is much, much more to cover about IBC security, runtime gas usage, numeric overflows, doing precise financial math with unsigned integers, writing safe Rust code, and avoiding panic which I'll fill in as time permits.