Thread by @pencilflip

.@armaniferrante recently published a great repository of 10 vulnerabilities to avoid when writing Solana programs.

Here's the quick rundown if you don't feel like digging into the code 👇

https://twitter.com/armaniferrante/status/1481378911065944073

1) Signer authorization

If your instruction takes in an "authority" account, make sure the account has signed the transaction.

Why? Because only the owner of the "authority" account can sign for it—but anyone can pass in the account as a non-signer.

Don't do this—authority is not required to be a signer.

Instead, do this—authority IS required to be a signer!

2) Account data matching

Make sure that passed-in accounts contain valid data.

For example, if your instruction expects a token account, the token account should contain an owner, mint, amount, etc.

Otherwise, you may be operating with the wrong type of account!

Don't do this—the token account can contain arbitrary, invalid data.

Instead, do this—Anchor checks that the token account contains valid data, and that its owner is the signer of the transaction.

3) Checking account ownership

Make sure the passed-in accounts are owned by the correct program.

For example, if your instruction expects a token account, it should be owned by the token program.

Don't do this—this code doesn't check to make sure the token account is owned by the SPL token program, so it could be invalid.

Instead, do this—Anchor will verify account ownership for you!

4) Type cosplay

Make sure one account type (e.g. User) can't be confused for another account type (e.g. Metadata).

This one is a bit confusing, the examples should make it clearer.

Don't do this—you can't tell a deserialized User account from a deserialized Metadata account

The manual way to fix this is by adding a "discriminant" to both accounts, i.e. something that allows you to distinguish between them

The recommended way to fix this is by just using Anchor's #[account] macro, which will automatically add an 8-byte discriminator to the start of the account. Much easier!

5) Account initialization

This is similar to the previous vulnerability—when initializing accounts, make sure you account for the discriminator.

E.g., you don't want to initialize the wrong type of account. And you may not want to re-initialize an already initialized account.

Don't do this—the user account could be another account type (since there is no discriminator).

And this also lets people re-initialize previously initialized accounts.

Instead, do this—using #[account(init)] will create a new account and set its account discriminator.

6) Arbitrary CPI

When performing CPIs, make sure you're invoking the correct program.

Don't do this—the token program account gets passed in by the user, and could actually be some other program.

The manual way to fix this is checking to make sure the token program account has the right address.

The recommended way to fix this is by using Anchor's wrapper of the SPL token program.

7) Duplicate mutable accounts

If your program takes in two mutable accounts of the same type, make sure people don't pass in the same account twice.

Don't do this—user_a and user_b may be the same account.

Instead, use Anchor to verify that the two accounts are different.

8) Bump seed canonicalization

Often, you want to have a single PDA associated with a program ID + a set of seeds.

For example, associated token accounts (ATAs).

Thus, when verifying PDAs, you should use find_program_address instead of create_program_address.

Don't do this—since the bump is passed in by the user (and not verified), set_value could operate on multiple PDAs associated with the program ID + the set of seeds.

Instead, do this—both the PDA address and bump are verified, which means set_value will only operate on one "canonical" PDA.

9) PDA sharing

You should use a unique PDA for each "authority domain" (H/T @armaniferrante).

The example in the repo is a bit confusing, so let's consider a different one.

Let's say you have a bunch of liquidity pools, and each one has a PDA as an authority.

Instead of using the same PDA as the authority for each pool, it's more secure to use a unique PDA for each pool (e.g. derived from the two token mints the pool is for).

10) Closing accounts

If you no longer need an account, you should close it to reclaim its rent.

However, it turns out that closing an account is pretty tricky.

I'm not going to walk through all the complexities, because there are a lot.

Simply put, if you want to close an account, just use this Anchor macro

Hopefully all this made sense! If I made any errors, or if you have any questions, please let me know.

Lastly, I hear @armaniferrante (who understands this stuff much better than me) is working on a presentation to explain all these vulnerabilities more thoroughly, so stay tuned for that 👀