Uninitialised local vars bug fixed

[bart1e]

Fixes #1505.
There is a bug in _parse_variable_definition when handling tuples in expressions like (type1 a, type2 b) = f().
It manifests itself when we have two variables named the same in different scopes and when they are defined in an expression like the one above.
The problem is that slither renames variables with the same names in different scopes, but while handling such expressions, an additional expression is created, but it contains old variable names. Code responsible for doing this is depicted below:

identifier = {
     "nodeType": "Identifier",
     "src": v["src"],
     "name": v["name"],
     "typeDescriptions": {"typeString": v["typeDescriptions"]["typeString"]},
}

It causes slither to think that the same variables are referenced in all scopes and it makes both slithir representation and further variable analysis incorrect.
I have suggested the simplest fix, that is, to modify the src variable so that the code executed afterwards (pasted above) creates an Expression with correct variable names. But, if we don't want to modify src, we may store all new variable names in a list and then use it when initialising the identifier variable.

[0xalpharush]

Can you add the example from OP's issue as a regression test and document why the "name" is being overwritten in a code comment please?

[bart1e]

@0xalpharush Sure, I will do that. Where should I put the test? In tests/slithir directory (checking whether the variables are renamed correctly), tests\detectors\uninitialised_local (to test just this detector behaviour) or somewhere else?

[0xalpharush]

Both would be awesome! But at least testing the detector behavior in tests\detectors\uninitialised_local would be great.

[bart1e]

@0xalpharush I have added both tests as requested.
I also tried to explain the fix in the comment, but I'm not sure it's clear - I tried to be concise, but it's hard to explain the bug without going deeply into the details. I have linked this PR in that comment just in case.

[duckki]

Oooh, I found this change worked, too.

CertiKProject@08faa90:

diff --git a/slither/solc_parsing/declarations/function.py b/slither/solc_parsing/declarations/function.py
index 13037521..f76fb23a 100644
--- a/slither/solc_parsing/declarations/function.py
+++ b/slither/solc_parsing/declarations/function.py
@@ -770,6 +770,7 @@ class FunctionSolc(CallerContextExpression):
                     for v in variables:
                         identifier = {
                             "nodeType": "Identifier",
+                            "referencedDeclaration": v["id"],
                             "src": v["src"],
                             "name": v["name"],
                             "typeDescriptions": {"typeString": v["typeDescriptions"]["typeString"]},

[0xalpharush]

Replaced by #1912