stacks-core icon indicating copy to clipboard operation
stacks-core copied to clipboard

VM: analysis pass should consider argument count when deploying a contract

Open jcnelson opened this issue 3 years ago • 1 comments

It was discovered in #2677 that the Clarity VM does not verify in the analysis pass that a function call contains the correct number of arguments. This is a consensus bug, so it cannot be fixed without a hard fork. We'll have to address it in Stacks 2.1, and it'll have to be gated. Specifically:

  • Contracts deployed after the epoch switch to Stacks 2.1 should fail analysis if they call a function with the wrong number of arguments
  • Contracts deployed before the epoch switch should continue to pass analysis, but if they would fail analysis in 2.1 due to this bug, they should be marked as such
  • (at-block ..) calls to Stacks 2.0 contracts from Stacks 2.1 contracts should fail analysis if the contract was tagged as buggy per the above.

jcnelson avatar Jun 01 '21 20:06 jcnelson

No remediation action is necessary. Attempts to contract-call? another contract's public function with the wrong number of arguments, or attempts to contract-call? a function that ultimately calls a function with the wrong number of arguments will both fail with UncheckedError, which invalidates the whole block.

$ cat /tmp/clarity-error.clar
(define-read-only (main-function (param1 uint) (param2 uint))
  (some u10)
)

;; here we call function using 3 arguments instead of 2
(define-read-only (first-function)
  (match (main-function u1 u2 u3)
    value (+ value u10)
    u0
  )
)

;; here we call function using only 1 argument instead of 2
(define-read-only (second-function)
  (main-function u1)
)
$ clarity-cli initialize /tmp/clarity-error.db
INFO [1622578955.200364] [src/vm/functions/mod.rs:445] [main] "``... to be a completely separate network and separate block chain, yet share CPU power with Bitcoin`` - Satoshi Nakamoto"
{"message":"Database created.","network":"mainnet"}
$
$
$ # bug manifests below -- this should have failed
$ clarity-cli launch SP1KWKM131TW5NAMX6X59A1ZZFVT731RYC9BD35X6.clarity-error /tmp/clarity-error.clar /tmp/clarity-error.db
{"events":[],"message":"Contract initialized!"}
$
$ cat /tmp/clarity-error-cc.clar
(begin
    (contract-call? 'SP1KWKM131TW5NAMX6X59A1ZZFVT731RYC9BD35X6.clarity-error first-function)
)
$ clarity-cli check /tmp/clarity-error-cc.clar /tmp/clarity-error.db
{"error":{"initialization":"Unchecked(IncorrectArgumentCount(2, 3))"}}
$ clarity-cli launch SP1KWKM131TW5NAMX6X59A1ZZFVT731RYC9BD35X6.clarity-error-cc-001 /tmp/clarity-error-cc.clar /tmp/clarity-error.db
{"error":{"initialization":"Unchecked(IncorrectArgumentCount(2, 3))"}}
$ clarity-cli launch SP1KWKM131TW5NAMX6X59A1ZZFVT731RYC9BD35X6.clarity-error-cc-001 /tmp/clarity-error-cc.clar /tmp/clarity-error.db
{"error":{"initialization":"Unchecked(IncorrectArgumentCount(2, 3))"}}

jcnelson avatar Jun 01 '21 20:06 jcnelson

Transactions with the wrong number of arguments will be mined in 2.1 because we convert all analysis errors to runtime errors.

jcnelson avatar Feb 22 '23 03:02 jcnelson

Assigning to @obycode so this issue has an owner. Please feel free to re-assign.

jcnelson avatar Feb 22 '23 03:02 jcnelson