singnet / snet-upgradeable-owners-minting-policy

0 stars 1 forks source link

Audit Meta #1

Open zmrocze opened 6 days ago

zmrocze commented 6 days ago

"SingularityNet Upgradeable Owners" Audit

Validators

Renegatto commented 4 days ago

My notes, including user and protocol requirements:

Design notes:

Some high-level notes and assumptions about the protocol design based on the spec overview. These are not clearly specified and may not match the actual implementation.

s - concrete protocol instance.

Quantity relations between entities:

1 Token MP s ↔ 1 UpdateableAddresses Validator s ↔ 1 NFT s ↔ 1 NFT MP s ↔ 0+ Token s

Scripts relations:

Token MP s -knows→ UpdateableAddresses Validator s -knows→ NFT MP s

Parametrized scripts:

User requirements

U0: I want to always be able to mint and burn existing tokens

Follows:

U4: I don't want to have any unspendable funds on the protocol scripts

U5: I want any user to be able to freely burn their tokens of this protocol

Preconditions

State thread is valid and ready

Protocol requirements

All the protocol requirements MUST imply preconditions.

P0: Every NFT token can ONLY be found at the only UpdateableAddresses validator at ANY moment of time

Follows from: U0

P1: The amount of signers MUST be odd

Follows from: U1, U2, U3

P2: Protocol UTxOs MUST contain only necessary assets and datum

Because of common vilnerabilities:

Follows from U0

P3: UpdateableAddresses MUST only allow spending of it's NFT in a Tx with a strict majority of signatures

Because of common vilnerabilities:

Follows from U3

P4: UTxO containing NFT token MUST have valid state thread datum

Because of common vilnerabilities:

Follows from U3, U0

Corresponding UpdateableAddresses validator MUST ensure that.

P5: Token MP MUST only allow minting in a Tx with the corresponding NFT being spent

Because of common vilnerabilities:

Follows from U3, U0

P6: UpdateableAddresses MUST allow spending of any UTxO that does not contain NFT

Because of common vilnerabilities:

Follows from U1, U4

P7: Protocol behaviour MUST not change regardless of any protocol token name change

Because of common vilnerabilities:

P8: Protocol MUST have correct arithmetic

Because of common vilnerabilities:

Follows from U0

P9: Protocol MUST not halt or stale

Follows from U0

P10: multiple-satisfaction is not possible

Because of common vilnerabilities:

It should not be possible, because:

P11: Burning tokens MUST not be constrained

P12: Burning tokens MUST not interfere with protocol state thread update

So it should not be possible to burn tokens and put a lot of trash tokens into a state thread UTxO in the same Tx.

Covered by P2, but just in case.

Because of common vilnerabilities:

TODO: consider staking credentials

zmrocze commented 4 days ago

look for problems with:

requirements

token scripts

  1. Can only mint a token if owners accept with a defined threshold.
  2. Can only mint if spending output with NFT.

NFT

  1. After creation, there is always single NFT token in existance.
  2. The token is held at UpgradeableAddresses script address.

UpgradeableOwners

User

  1. Protocol allows addition or removal of owners provided the minimum number of owners agree.
  2. Protocol allows the changing of the minimum required number of signatures for acceptance, provided the minimum number of owners agree.
  3. Any Token owner can freely burn the tokens.
  4. Protocol allows minting of new Tokens provided the minimum number of owners agree.
  5. As a user I cannot steal Tokens (meaning they land where agreed by threshold).

Protocol

  1. The NFT containing output stores the minimum threshold and owners' addresses.
  2. The minimum threshold is no greater than the number of owners and greater than 1.
  3. TODO: The number of owners is ___.
  4. Token minting transaction doesn't change owners and threshold.
rmgaray commented 4 days ago

Goal:

To mint tokens safely and allow holders of these tokens to burn them freely.

Requirements:

  1. To mint the asset, there must be at least min_threshold signatures from the minters_set.
  2. Any owner of the asset must be able to burn it if desired.
  3. The minters_set must be updatable. Any addition/removal of owners to/from this set must have min_threshold signatures from the minters_set.
  4. The minters_set must have at least 2 members.
  5. The min_threshold must be updatable. Any change to this number must have min_threshold signatures from the minters_set.
  6. The min_threshold must always be within the range of [2; |minters_set|]

Questions:

What if the same public key hash is used more than once in the datum? Depending on the implementation, this could be a problem.

groscoe commented 4 days ago

User actions