crytic / crytic/slither

[Bug]: Dangling temporary variable in array push return variables (solidity <0.6.0)

Open
#1,793 1 comment 0 reactions 0 assignees View on GitHub
bug ir
Dominant language
Python
Stars
6.4k
Forks
1.1k
PR merge metrics
No merged PRs in 30d

Description

### Describe the issue:

At `solidity < 0.6.0`, the push method of array variables has a return value which is the new array length after push.
However, slither does not properly handle this case.
The temporary variable (`TMP_0` in the following example), which refers to the new array length, comes out of nowhere, and its type is None.

There should be some IR operations like
```
TMP_0(uint256) -> LENGTH arr
```
which defines `TMP_0`.

Alternatively, the previous `REF_1` can be also used to be assigned to `x(uint256)`.

### Code example to reproduce the issue:

```solidity
contract A {
uint[] arr;
uint x;
function foo() public {
x = arr.push(1);
}
}

```

### Version:

0.9.3

### Relevant log output:

```shell
Contract A
Function A.foo() (*)
Expression: x = arr.push(1)
IRs:
REF_1 -> LENGTH arr
TMP_1(uint256) := REF_1(uint256)
TMP_2(uint256) = TMP_1 + 1
REF_1 (->arr) := TMP_2(uint256)
REF_2(uint256) -> arr[TMP_1]
REF_2 (->arr) := 1(uint256)
x(uint256) := TMP_0(None) # Buggy, TMP_0 comes out of nowhere with None type
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.