crytic / slither

Static Analyzer for Solidity and Vyper
https://blog.trailofbits.com/2018/10/19/slither-a-solidity-static-analysis-framework/
GNU Affero General Public License v3.0
5.27k stars 964 forks source link

[Bug]: Missing quotes in case mapping key passed as a string #2538

Open WaizKhan7 opened 1 month ago

WaizKhan7 commented 1 month ago

Describe the issue:

We are using Slither Python API to analyze the expression of each node of function in a contract within Slither object. The issue I need help with is that Slither expression does not differentiate between the following cases (in Solidity): mapping(string => uint256) stringGeneBuckets;

//case # 1
stringGeneBuckets["myvariable"] = _geneId;
//case # 2
myvariable = "someStringValue"
stringGeneBuckets[myvariable] = _geneId;

For both cases mentioned above, the node.expression returned is stringGeneBuckets[myvariable] = _geneId, in the first case shouldn't it be stringGeneBuckets["myvariable"] = _geneId?

Code example to reproduce the issue:

pragma solidity ^0.8.0;

contract GeneMapping {

// Mapping from a string to a uint256 value
mapping(string => uint256) public stringGeneBuckets;

function setGeneId(string memory _myvariable, uint256 _geneId) public {
    stringGeneBuckets["myvariable"] = _geneId;
    stringGeneBuckets[_myvariable] = _geneId;
}

// Function to retrieve a value from the mapping for a given key
function getGeneId(string memory key) public view returns (uint256) {
    return stringGeneBuckets[key];
}

}

Version:

0.9.6

Relevant log output:

No response

0xalpharush commented 1 month ago

Seems like a quote isn't being escaped. What are you trying to accomplish? There's probably a better way to do it than comparing the expression (which just casts the AST expression with str) e.g. you can tell a literal from an assignment in the Slither API. The source mapping API also will give you the source as it appears in the file https://github.com/crytic/slither/blob/aeeb2d368802844733671e35200b30b5f5bdcf5c/slither/core/source_mapping/source_mapping.py#L72-L82

WaizKhan7 commented 1 month ago

You are right. In these specific cases, it is possible to use variables_read to differentiate. But it gets tricky if the code is something like:

    mapping(string => uint256[]) public stringGeneBuckets;

    function setGeneId(string memory myvariable, uint256 _geneId) public {
        stringGeneBuckets["myvariable"].push(_geneId);
        stringGeneBuckets[myvariable].push(_geneId);
    }

In this case, the expression is treated as a function_call, and I can't find a way to differentiate b/w them.