add descriptions for all type-cosplay examples

Author: oslfmt

lints/type_cosplay/README.md

@@ -34,14 +34,72 @@ or u8, and not an enum. This will flag a false positive.
 
 ## Note on Tests
 
-**insecure-anchor**: insecure because `User` type derives Discriminator trait (via `#[account]`),
+### insecure
+
+This is the canonical example of type-cosplay. The program tries to deserialize
+bytes from `AccountInfo.data` into the `User` type. However, a malicious user could pass in
+an account that has in it's data field the `Metadata` type. This type is equivalent to the
+`User` type, and the data bytes will thus successfully deserialize as a `User` type. The
+program performs no checks whatsoever, and will continue on operating with a pubkey that it
+believes to be a `User` pubkey, not a `Metadata` pubkey.
+
+### insecure-2
+
+This is insecure because the program tries to deserialize from multiple enum types.
+Here, `UserInfo` and `MetadataInfo` enums are both being deserialized. Note that both of these
+enums contain a single variant, with the struct type nested inside it. This evades the in-built
+discriminant of an enum. A `Metadata` type could be deserialized into a `UserInfo::User(User)`,
+and a `User` could be deserialized into a `MetadataInfo::Metadata(Metadata)`.
+
+Only deserializing from a single enum is safe since enums contain a natural, in-built discriminator.
+If _all_ types are nested under a variant of this enum, then when deserializing, the enum variant
+must be matched first, thus guaranteeing differentiation between types.
+
+However, deserializing from multiple enums partitions the "set of types" and is thus not exhaustive
+in discriminating between all types. If multiple enums are used to encompass the types, there may
+be two equivalent types that are variants under different enums, as seen in this example.
+
+### insecure-3
+
+This example is insecure because `AccountWithDiscriminant` could be deserialized as a
+`User`, if the variant is `Extra(Extra)`. The first byte would be 0, to indicate the discriminant
+in both cases, and the next 32 bytes would be the pubkey. The problem here is similar to
+the insecure-2 example--not all types are nested under a single enum type. Except here,
+instead of using another enum, the program also tries to deserialize `User`.
+
+This illustrates that in order to properly take advantage of the enums natural built-in
+discriminator, you must nest _all_ types in your program as variants of this enum, and
+only serialize and deserialize this enum type.
+
+### insecure-anchor
+
+Insecure because `User` type derives Discriminator trait (via `#[account]`),
 thus one may expect this code to be secure. However, the program tries to deserialize with
 `try_from_slice`, the default borsh deserialization method, which does _not_ check for the
 discriminator. Thus, one could potentially serialize a `Metadata` struct, and then later
 deserialize without any problem into a `User` struct, leading to a type-cosplay vulnerability.
 
-**recommended**: this is secure code because all structs have an `#[account]` macro attributed
+### recommended
+
+This is secure code because all structs have an `#[account]` macro attributed
 on them, thus deriving the `Discriminator` trait for each. Further, unlike the insecure-anchor
 example, the program uses the proper deserialization method, `try_deserialize`, to deserialize
 bytes as `User`. This is "proper" because in the derived implementation of `try_deserialize`,
 the discriminator of the type is checked first.
+
+_Note: this example differs from the Sealevel [recommended](https://github.com/coral-xyz/sealevel-attacks/blob/master/programs/3-type-cosplay/recommended/src/lib.rs) example in that it actually attempts_
+_to perform a deserialization in the function body, and then uses the struct. It provides_
+_a more realistic and concrete example of what might happen in real programs_
+
+### secure
+
+This example is from the Sealevel [example](https://github.com/coral-xyz/sealevel-attacks/blob/master/programs/3-type-cosplay/secure/src/lib.rs). It fixes the insecure case by adding a `discriminant`
+field to each struct, and further this discriminant is "proper" because it contains the
+necessary amount of variants in order to differentiate each type. In the code, there is
+an explicit check to make sure the discriminant is as expected.
+
+### secure-2
+
+This example fixes both the insecure and insecure-2 examples. It is secure because it only deserializes
+from a single enum, and that enum encapsulates all of the user-defined types. Since enums contain
+an implicit discriminant, this program will always be secure as long as all types are defined under the enum.

lints/type_cosplay/ui/insecure-2/src/lib.rs

@@ -3,16 +3,6 @@ use borsh::{BorshDeserialize, BorshSerialize};
 
 declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS");
 
-// NOTE: this is insecure because the program tries to deserialize from multiple enum types.
-// This is a vulnerability because it partitions the "set of types" and is thus not exhaustive
-// in discriminating between all types. Deserializing from a single enum is safe because the
-// program will have to match the deserialized result against an enum variant in order to reconstruct
-// the ADT. This effectively serves as a discriminant, because even if two types deserialize the same,
-// the programmer will explicitly match it to a type.
-// If multiple enums are used to encompass the types, this defense will basically be breached.
-// There may be two different types that are equivalent. If we deserialize from an enum,
-// type B could be passed in, when what was expected was type A.
-
 // NOTE: what about the case where we only deserialize from one enum? Could this still be a vulnerability?
 // ex. only deser from UserInfo. Someone sets data as MetadataInfo::Metadata(...). will this match?
 // what if MetadataInfo::User(...)?

lints/type_cosplay/ui/insecure-2/src/lib.stderr

@@ -1,12 +1,12 @@
 error: multiple enum types deserialized. Should only have one enum type to avoid possible equivalent types
-  --> $DIR/lib.rs:27:20
+  --> $DIR/lib.rs:17:20
    |
 LL |         let user = UserInfo::try_from_slice(&ctx.accounts.user.data.borrow()).unwrap();
    |                    ^^^^^^^^
    |
    = note: `-D type-cosplay` implied by `-D warnings`
 help: consider constructing a single enum that contains all type definitions as variants
-  --> $DIR/lib.rs:41:24
+  --> $DIR/lib.rs:31:24
    |
 LL |         let metadata = MetadataInfo::try_from_slice(&ctx.accounts.user.data.borrow()).unwrap();
    |                        ^^^^^^^^^^^^

lints/type_cosplay/ui/insecure-3/src/lib.rs

@@ -3,9 +3,6 @@ use borsh::{BorshDeserialize, BorshSerialize};
 
 declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS");
 
-// This example is insecure because AccountWithDiscriminant could be deserialized as a
-// User, if the variant is Extra(Extra). The first byte would be 0, to indicate the discriminant
-// in both cases, and the next 32 bytes would be the pubkey.
 #[program]
 pub mod type_cosplay_secure {
     use super::*;

lints/type_cosplay/ui/insecure-3/src/lib.stderr

@@ -1,12 +1,12 @@
 error: Deserializing from different ADT types.
-  --> $DIR/lib.rs:16:20
+  --> $DIR/lib.rs:13:20
    |
 LL |         let user = User::try_from_slice(&ctx.accounts.user.data.borrow()).unwrap();
    |                    ^^^^
    |
    = note: `-D type-cosplay` implied by `-D warnings`
 help: deserialize from only structs with a discriminant, or an enum encapsulating all structs.
-  --> $DIR/lib.rs:29:13
+  --> $DIR/lib.rs:26:13
    |
 LL |             AccountWithDiscriminant::try_from_slice(&ctx.accounts.user.data.borrow()).unwrap();
    |             ^^^^^^^^^^^^^^^^^^^^^^^

lints/type_cosplay/ui/secure-2/src/lib.rs

@@ -3,9 +3,6 @@ use borsh::{BorshDeserialize, BorshSerialize};
 
 declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS");
 
-// This example is secure because it only deserializes from a single enum, and that enum
-// encapsulates all of the user-defined types. Since enums contain an implicit discriminant,
-// this program will always be secure as long as all types are defined under the enum.
 #[program]
 pub mod type_cosplay_secure {
     use super::*;