diwakergupta / stacks-blockchain-tob-audit

GNU General Public License v3.0
0 stars 0 forks source link

Question re mutually recursive contracts #19

Open smoelius opened 4 years ago

smoelius commented 4 years ago

I added the below test_mutually_recursive_contracts here: https://github.com/trailofbits/x-audit-blockstack-core/blob/e2d3d5bae539d242851620e28129af6c4a9de642/src/vm/tests/contracts.rs#L685 and it passes, though it seems like it shouldn't. Is this "cheating" in some way, e.g., is there analysis pass that is being skipped?

fn test_mutually_recursive_contracts(owned_env: &mut OwnedEnvironment) {
    let contract_a =
        "(define-read-only (factorial (a int))
           (contract-call? .contract-b factorial a))";
    let contract_b =
        "(define-read-only (factorial (a int))
           (if (is-eq a 0)
             1
             (* a (contract-call? .contract-a factorial (- a 1)))))";

    let p1 = execute("'SZ2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKQ9H6DPR");

    {
        let mut env = owned_env.get_exec_environment(None);
        env.initialize_contract(QualifiedContractIdentifier::local("contract-a").unwrap(), contract_a).unwrap();
        env.initialize_contract(QualifiedContractIdentifier::local("contract-b").unwrap(), contract_b).unwrap();
    }

    {
        let c_b = Value::from(PrincipalData::Contract(QualifiedContractIdentifier::local("contract-b").unwrap()));
        let mut env = owned_env.get_exec_environment(Some(p1.clone()));
        assert_eq!(
            env.execute_contract(&QualifiedContractIdentifier::local("contract-b").unwrap(), "factorial", &vec![SymbolicExpression::atom_value(Value::Int(10))][..]).unwrap(),
            Value::Int(3628800));
    }
}
kantai commented 4 years ago

Yes -- the static analysis will error on this, and the tests in vm::tests are intended to circumvent the analysis to test how the VM behaves with code that hasn't been analyzed. In any case, the VM should be able to catch these kinds of recursive calls, so this is a bug.

lgalabru commented 4 years ago

I think I hit this issue while working on traits and fixed it - https://github.com/blockstack/blockstack-core/commit/8faade53f7507964dfe9475af0f028d6e6293ecb