code-423n4 / 2023-08-pooltogether-findings

4 stars 3 forks source link

The @dev comments states that the setOriginChainOwner() function can only be called once, but it seems it can be called multiple times #131

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/generationsoftware/remote-owner/blob/9c093dbd36c1f18ab7083549d10ac601d91630df/src/RemoteOwner.sol#L91-L109

Vulnerability details

Impact

The @dev comments states that the setOriginChainOwner() function can only be called once, but it seems it can be called multiple times.

If the function can be called more than once as seems to be the case currently, then it could open up vulnerabilities.

It seems the function itself is implemented correctly, but the problem arises in the constructor of the RemoteOwner.sol contract, where the _setOriginChainOwner(__originChainOwner); is not explicitly set to the msg.sender of the constructor function.

Proof of Concept

n/a

Tools Used

VSC, manual.

Recommended Mitigation Steps

Get the checks/assignments of the constructor in line with the checks/assignments of the setOriginChainOwner() function, so that "@dev Can only be called once." is true in all cases.

Assessed type

Access Control

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

raymondfam commented 1 year ago

QA at best. And, it's checked here:

https://github.com/generationsoftware/remote-owner/blob/9c093dbd36c1f18ab7083549d10ac601d91630df/src/RemoteOwner.sol#L117-L121

c4-judge commented 1 year ago

HickupHH3 marked the issue as duplicate of #40

c4-judge commented 1 year ago

HickupHH3 changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

HickupHH3 marked the issue as grade-c