MyCut

Summary

Both the Pot and ContestManager smart contracts incorrectly inherit the Ownable contract. The Ownable constructor is being called with an argument (msg.sender), which is not necessary and deviates from standard practices. This issue could potentially lead to unexpected behavior or make the contracts less readable and harder to maintain.

Vulnerability Details

The Ownable contract from OpenZeppelin’s library already has a constructor that initializes the contract’s owner to the address that deploys the contract. Passing msg.sender to Ownable is redundant and can lead to confusion or unintended behavior if the intention was to have a different owner initialization. Inheriting Ownable without passing any arguments to its constructor is the correct approach. The Ownable contract’s constructor automatically sets the owner to the contract deployer.

Impact

The incorrect inheritance pattern makes the code less readable and can lead to maintenance challenges. If misunderstood or modified incorrectly, this could potentially lead to a situation where ownership is assigned incorrectly during deployment.

Tools Used

Manual Review

Recommendations

To resolve this issue, the inheritance of Ownable should be corrected as follows:

In Pot Contract:

Current Code:

Updated Code:

In ContestManager Contract:
Current Code:

Updated Code:

By removing the unnecessary passing of msg.sender, the contracts will adhere to best practices, improving both security and maintainability. This change ensures that the contracts correctly inherit ownership functionality from OpenZeppelin's Ownable contract.