minswap / minswap-dex-v2 Goto Github PK

Suggested by @Micrograx and @micahkendall

• if someone creates a license authorisation which uses a spend authorisation, they would be unwittingly authorising the use of that spend authorisation as a stake authorisation
• the problem comes in because all spend scripts can be used to mint or withdraw 2-arg validators arbitrarily
• so, someone could register that spend script to receive staking rewards, and then anyone could withdraw 0 in order to be a licensed operator
• because you reuse authorize_license_holder for the admin authorisation, it is possible depending on the design of how you were going to authorise the admin, that this would be a critical vulnerability
• you can prevent this in two ways: either be extremely careful with the offchain, never use a spend script without using a multivalidator which ensures that 2-arg parameters are rejected, or you can make an adjustment to how the credential is passed

The solution is replace the "Address" type in GlobalSetting to "PoolAuthorizationMethod"

pub type PoolAuthorizationMethod {
  PAMSignature { pub_key_hash: PubKeyHash }
  PAMSpendScript { script_hash: ValidatorHash }
  PAMWithdrawScript { script_hash: ValidatorHash }
}

The hash length should be validated whenever a new GlobalSetting UTxO is created

Suggested by @micahkendall

I believe factory may be vulnerable to a DOS attack on the value field. Depending on costing parameters, this could either make it more expensive to insert factories or it could make it greater than the budget which would break insertions

I'm going to double check and write a description

Here is the attack:

• offchain, define an "attacking policy" as a native script with a random salt, that happens to put the policy id as being lesser than the authen policy id
• generate as many of these policies as necessary, with the asset name being empty and the quantity being 1
• insert these assets into the value along with the factory NFT, when a new DEX is made.
• because the quantity_of has to be a linear search left to right over the list, this operation has to skip over every policy that has been inserted, consuming exunits
• depending on how much this costs, this could cause the budget to be exceeded, which would freeze the ability of minswap to insert more dexes
• to validate this attack, we would have to consider the memory usage of the quantity_of operation for large numbers of policies, and whether the necessary count of policies can actually fit in a transaction which constructs a valid factory
• to resolve this attack, simply limit the number of values in the factory value to the maximum necessary
• Also, on a technicality, you have to mint all the attacking assets in a transaction before the attack, because the tx prevents minting other assets, but it's still viable because you can just spend the assets from a user input from a previous tx