`references` for virtual functions are filled incorrectly

[bart1e]

Describe the issue:

Suppose that we have a contract A defined in A.sol that has a virtual function f and a (normal) function g that uses f. Additionally, there is a contract B inheriting from A and overriding f.

Then, references list for B.f contains a code from A.sol (the line where f is referenced in g).

Here is the code fragment that updates references list: https://github.com/crytic/slither/blob/3383e39823a088fc00c6a2563ba23ac202b29c39/slither/solc_parsing/expressions/expression_parsing.py#L465-L467

In this case, identifier.source_mapping will reference a fragment of g function from A.sol and references list will be updated incorrectly.

Code example to reproduce the issue:

// content of A.sol:
pragma solidity 0.7.6;

contract A
{
    function f() internal virtual {}

    function g() internal
    {
        f();
    }
}

//content of B.sol:
pragma solidity 0.7.6;

import "./A.sol";

contract B is A
{
    function f() internal override {} // this function will have A.sol#9 in `references`
}

Version:

0.9.2

Relevant log output:

No response

[bart1e]

After giving it more thought, I think that it might be a correct Slither behaviour, because g in this case belongs to the contract B (since it inherits from A), but is defined in the A.sol, so source_mapping cannot point to some code in B.sol. Hence, in fact, there is no better candidate for source_mapping here.

If this is the case, then the issue may be closed.

[0xalpharush]

@bart1e Is this different from what is outlined here https://github.com/crytic/slither/blob/4c976d5af56219eeef079e03a35009af3e03644d/slither/solc_parsing/expressions/find_variable.py#L420-L439

[bart1e]

@0xalpharush I think it's sligthly different since I was referring to the use (not the declaration) of function f in g in the example I gave.

It was confusing to me since it seemed strange that a function from file A can reference function from file B, when file A doesn't import B.

But when I think about it, there is just no better candidate for reference of the function f from my example. So, while it was confusing at first, I think now that this is correct behaviour of Slither.