Consensys/scribble

Scribble breaks on contracts with functions returning pointers

cd1m0 opened this issue · 0 comments

cd1m0 commented

Given the following sample:

contract Foo {
    /// #if_updated arr.length > 0;
    uint[] arr;

    int8[] arr1;
    function getPtr() internal returns (int8[] storage) {
        return arr1;
    }

    function main() {
        getPtr().push(0);

        arr.push(1);
    }
}

Scribble will throw the following error:


rror: Unexpected LHS.

FunctionCall #28
    id: 28
    src: "287:8:0"
    typeString: "int8[] storage pointer"
    kind: "functionCall"
    fieldNames: undefined
    vExpression: Identifier #27
    vArguments: Array(0)
    context: ASTContext #1
    parent: MemberAccess #29
    <getter> children: Array(1) [ Identifier #27 ]
    <getter> vIdentifier: "getPtr"
    <getter> vMemberName: undefined
    <getter> vFunctionCallType: "userDefined"
    <getter> vReferencedDeclaration: FunctionDefinition #15
    <getter> vFunctionName: "getPtr"
    <getter> vCallee: Identifier #27
    <getter> type: "FunctionCall"
    <getter> firstChild: Identifier #27
    <getter> lastChild: Identifier #27
    <getter> previousSibling: undefined
    <getter> nextSibling: undefined
    <getter> root: SourceUnit #36
    <getter> sourceInfo: Object { offset: 287, length: 8, sourceIndex: 0 }

    at assert (/home/dimo/work/consensys/scribble/node_modules/solc-typed-ast/dist/misc/utils.js:27:11)
    at decomposeLHS (/home/dimo/work/consensys/scribble/dist/instrumenter/state_vars.js:425:33)
    at addStateVarUpdateLocDesc (/home/dimo/work/consensys/scribble/dist/instrumenter/state_vars.js:503:33)
    at findStateVarUpdates (/home/dimo/work/consensys/scribble/dist/instrumenter/state_vars.js:553:13)
    at instrumentFiles (/home/dimo/work/consensys/scribble/dist/bin/scribble.js:251:70)
    at /home/dimo/work/consensys/scribble/dist/bin/scribble.js:644:13

The issue is that the decomposeLHS function expects all LHS of assignments to be decomposable to Identifiers or MemberAccesses, but the following assignment LHS is a function call:

      getPtr().push(0);

In this case this is benign, as the state variable referenced by the pointer is not the one we are instrumenting. However the decomposeLHS code is called before instrumentation, to gather all update sites for state variables.

I think its safe to allow decomposeLHS to also return FunctionCalls as root, and just ignore them, since:

  1. if any state variable is being instrumented, then its pointer should never be leaked
  2. The logic for discovering which pointers are leaked doesn't call decomposeLHS
  3. If a state variable doesn't have its pointer leaked, then that pointer can't be returned by a function