sherlock-audit / 2024-06-magicsea-judging

8 stars 5 forks source link

Tricky Pebble Dachshund - IVoter is declared as storage variable and is initialized in constructor in MasterChef contract #695

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 4 months ago

Tricky Pebble Dachshund

Low/Info

IVoter is declared as storage variable and is initialized in constructor in MasterChef contract

Summary

MasterChef is an upgradeable contract and hence the state initialisation should be done in the proxy storage and not the implementation contract. But, the current implementation initialises the IVoter in the constructor of MasterChef which is not useful for the protocol. The MasterChef has setVoter function to set the value on proxy storage which addresses the concern.

Vulnerability Detail

IVote variable is declared as private storage level variable, while there is a comment to make the variable as immutable. Making the variable as immutable ensures that the value is initialized in the constructor and is part of the contract's byte code.

But that is not case in the current implementation leaving the possibility for IVote variable to be left uninitialized.

Impact

If IVote was not initialized via set functions, it could create temporary issues of reverting.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MasterchefV2.sol#L274-L276

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MasterchefV2.sol#L460-L466

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MasterchefV2.sol#L477-L479

Tool used

Manual Review

Recommendation

a) Make the IVote variable as immutable and hence initialization in constructor will work b) Alternatively, set IVote in initialize() function in the implementation contract.