SSA Operations using wrong variable for function args?

[bsamuels453]

I have the following deposit function:

    function deposit(uint256 amount, address receiver) public returns (uint256 sharesToMint) {
        uint256 supplyOffset = totalSupply + 10**offset;
        sharesToMint = amount * supplyOffset / (totalAssets() + 1);
        _mint(sharesToMint, receiver);
    }

This function builds the following slithir ssa operations list:

I wanted to detect when sharesToMint (ssa: sharesToMint_1) is tainted by user input.

In the example function, sharesToMint is clearly tainted by the user input amount, but when I run is_tainted_ssa() on sharesToMint_0, the value shows up as untainted.

Interestingly enough, there is no phi linking amount_0 (the value that slither knows is tainted) and amount_1 (the value that is actually used in the ssa operations list).

Is this a bug, or am I misunderstanding the purpose of is_tainted_ssa?

[bsamuels453]

heres a detector for debugging:

def _detect(self):
    results = []

    for contract in self.contracts:
        if not contract.name == "VaultImpl":
            continue
        for function in contract.functions:
            if function.signature_str != "deposit(uint256,address) returns(uint256)":
                continue
            # assuming the last call is _mint for this test code
            mint_operation = function.slithir_operations[-1]
            shares_to_mint_var = mint_operation.arguments[0]

            # check taint
            tainted = is_tainted_ssa(shares_to_mint_var, contract)
            print(tainted)
            return []
    assert(False);
    return []

[montyly]

SlithIR has two representation, the SSA and the non-SSA one. So here if you use is_tained_ssa you should use slithir_ssa_operations. Most of our detectors don't actually direct use the SSA representation.

For completion, in practice, Slither uses even a third representation, which is a temp representation when converting Solidity to SlithIR.

However there seems to be an issue is the SSA index, where the function parameters start at the index 1, while they should start at the index zero. I created an issue to track this here: #1683

[montyly]

I would also recommend to change

        for function in contract.functions:
            if function.signature_str != "deposit(uint256,address) returns(uint256)":
                continue

With contract.get_function_from_signature("deposit(uint256,address)") ;)