Make <Modifying storage array by value> detector more accurate
- Dominant language
- Python
- Stars
- 6.4k
- Forks
- 1.1k
- PR merge metrics
- No merged PRs in 30d
Description
### Describe the issue:
When I was reading this [detector](https://github.com/crytic/slither/wiki/Detector-Documentation#modifying-storage-array-by-value)'s source code, I found it will misreport the test case below:
```solidity
contract Memory {
uint[1] public x; // storage
uint[1] public y; // storage
function f() public {
uint[1] memory temp;
f1(x); // update x
f2(temp, y); // update temp & y
}
function f1(uint[1] storage arr) internal { // by reference
arr[0] = 1;
}
function f2(uint[1] memory arr, uint[1] storage arr2) internal { // by value
arr[0] = 2;
arr2[0] = 3;
}
}
```
For this case, it will not modify storage array `y` (also memory array `temp`) by value. But the output is
```
Memory.f() (arrayTest.sol#5-9) passes array Memory.y (arrayTest.sol#3)by reference to Memory.f2(uint256[1],uint256[1]) (arrayTest.sol#15-18)which only takes arrays by value
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#modifying-storage-array-by-value
arrayTest.sol analyzed (1 contracts with 1 detectors), 1 result(s) found
```
I think the function [`get_funcs_modifying_array_params`](https://github.com/crytic/slither/blob/master/slither/detectors/compiler_bugs/array_by_reference.py#L58) also needs to record the indexes of array parameters (storage or not) in the parameter list, which would help it avoid this misreport.
And I wish I could try to help fix the problem.
Thanks a lot.
### Code example to reproduce the issue:
```solidity
contract Memory {
uint[1] public x; // storage
uint[1] public y; // storage
function f() public {
uint[1] memory temp;
f1(x); // update x
f2(temp, y); // update temp & y
}
function f1(uint[1] storage arr) internal { // by reference
arr[0] = 1;
}
function f2(uint[1] memory arr, uint[1] storage arr2) internal { // by value
arr[0] = 2;
arr2[0] = 3;
}
}
```
### Version:
0.4.24
### Relevant log output:
_No response_
Contributor guide
Assessment
This issue has not been assessed yet.