crytic / crytic/slither

[Bug-Candidate]: shadow private variable

Open
#1,440 7 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Python
Stars
6.4k
Forks
1.1k
PR merge metrics
No merged PRs in 30d

Description

### Describe the issue:

I made a small change to the [state-variable-shadow](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variable-shadowing) example. If the variable `_owner` in BaseContract is decorated by private and it cannot be inherited by DerivedContract. The `isOwner` in BaseContract will work properly.

## Suggestion
I suggest that the `shadowing-state` detector exclude private variable shadowing.

## When this case:

When I want to inherit the [openzepplin ERC1155 contract](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC1155/ERC1155.sol) and override the `_uri` private variable as I would set a different URI for each tokenId and use other feature from oz's contract.

### Code example to reproduce the issue:

```solidity
pragma solidity ^0.8.0;

contract BaseContract {
address private _owner;

modifier isOwner() {
require(_owner == msg.sender);
_;
}
}

contract DerivedContract is BaseContract {
address private _owner;

constructor() {
_owner = msg.sender;
}

function withdraw() external isOwner {
payable(msg.sender).transfer(0);
}
}
```

### Version:

0.9.0

### Relevant log output:

```shell
Compilation warnings/errors on 1.sol:
Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: " to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.
--> 1.sol

DerivedContract._owner (1.sol#13) shadows:
- BaseContract._owner (1.sol#4)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#state-variable-shadowing

Pragma version^0.8.0 (1.sol#1) allows old versions
solc-0.8.17 is not recommended for deployment
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-versions-of-solidity

BaseContract._owner (1.sol#4) should be constant
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-constant
1.sol analyzed (2 contracts with 81 detectors), 4 result(s) found
```

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.