State Variable Shadowing

[code423n4]

Lines of code

https://github.com/jbx-protocol/juice-contracts-v2/blob/main/contracts/system_tests/TestAllowance.sol#L16
https://github.com/jbx-protocol/juice-contracts-v2/blob/main/contracts/system_tests/helpers/TestBaseWorkflow.sol#L52

Vulnerability details

Impact

Check: shadowing-state
Severity: High
Confidence: High

It is possible to use the same variable twice in Solidity, but it can lead to unintended side effects.The TestAllowance.sol contract inherits from TesTBaseWorkflow.sol. In the TestAllowance.sol contract, address _beneficiary; will shadow state variable in the TesTBaseWorkflow.sol contract. This will lead to major complications since the TesTBaseWorkflow.sol contract initializes the address beneficiary as follows: address private _beneficiary = address(69420); Overall, this can lead to the variable to not be assigned or to simply not work as intended.

Proof of Concept

--TestAllowance._beneficiary (contracts/system_tests/TestAllowance.sol#16) shadows:
- TestBaseWorkflow._beneficiary (contracts/system_tests/helpers/TestBaseWorkflow.sol#52)

Example of vulnerability:
contract BaseContract{

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

contract DerivedContract is BaseContract{

address owner;
constructor(){
    owner = msg.sender;
}

function withdraw() isOwner() external{
    msg.sender.transfer(this.balance);
} }

**owner of BaseContract is never assigned and the modifier isOwner does not work.

reference:
https://swcregistry.io/docs/SWC-119
ethereum/solidity#2563

Tools Used

Hardhat,remix

Recommended Mitigation Steps

Remove the state variable shadowing.

[mejango]

test contracts in scope? hm