Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix vulnerability in DParameterMP #134

Open
colll78 opened this issue Oct 3, 2024 · 0 comments
Open

Fix vulnerability in DParameterMP #134

colll78 opened this issue Oct 3, 2024 · 0 comments

Comments

@colll78
Copy link

colll78 commented Oct 3, 2024

Currently there is a vulnerability by which the operator burns and mints in the same transaction which allows him to by-pass the validator's checks that all tokens must go to the dParameter validator and smuggle tokens out of the contracts. This is due to the contract's dangerous use of currencySymbolValueOf.

I suggest you introduce the following checks to the DParameterMP:

  1. Each output to the dParameter validator address:
    i. contains exactly 1 DParameterToken
    ii. contains an inline datum that correctly decodes into DParameterValidatorDatum
           case (txOutDatum datum) of 
             OutputDatum d -> 
               let fields = (BI.snd . BI.unsafeDataAsConstr) d 
                   permissionedCandidatesCount = BI.head fields
                   pccTail = BI.tail fields 
                   registeredCandidatesCount = BI.head pccTail
               in (BI.null pccTail) && (BI.unsafeDataAsI permissionedCandidatesCount > 0) && (BI.unsafeDataAsI registeredCandidatesCount > 0)
  1. The transaction either mints or burns tokens, it does not do both. The current use of currencySymbolValueOf allows for an exploit where the operator burns and mints DParameter tokens in the same transaction thus tricking the script into believing that the number of tokens minted is less than it actually is. Instead of currencySymbolValueOf you should use variants that only sums the quantities that are positive and another variant that only sums the quantities which are negative and then you can check that the tx is only minting or only burning ie:
(positiveSymbolValueOf txInfoMint > 0 && negativeSymbolValueOf txInfoMint == 0) || (negativeSymbolValueOf txInfoMint < 0 && positiveSymbolValueOf txInfoMint == 0)

or just use variants onlyPositiveValueOf and onlyNegativeValueOf that error upon encountering non-positive / non-negative quantities and then use two redeemers (MintAct / BurnAct) to branch on minting and burning. The PermissionedCandidatesMint minting policy also suffers from this exploit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant