code-423n4 / 2021-06-pooltogether-findings

0 stars 0 forks source link

Inconsistent usage of ` _msgSender()` and `msg.sender` #110

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

shw

Vulnerability details

Impact

In some contracts such as PrizePool, the Openzeppelin's implementation _msgSender() is used to get the sender, while in other contracts such as *YieldSource, msg.sender is directly used instead. Inconsistency usage could make the codebase more difficult to maintain and could possible introduce errors if the _msgSender() is overridden to other implementations in the future.

Proof of Concept

Referenced code: Please use grep -R '_msgSender()' . and grep -R 'msg\.sender' . to find those contracts.

Recommended Mitigation Steps

Change all msg.sender to _msgSender(). Make the yield source contracts inherit from Openzeppelin's ContextUpgradeable if necessary.

asselstine commented 3 years ago

These are actually five different code bases. If there was an internal inconsistency in one I agree that this would be a problem; however the yield sources each live independently in their own projects.

dmvt commented 3 years ago

Agree with the sponsor on this one. Insistent use across multiple code bases is not an issue nor does it represent a vulnerability vector.