feeRecipient can be set to address(0) if the feeMantissa variable is greater than 0
Summary
An operator is able (accidentally or on purpose) to set the feeRecipient address to the zero address if the feeMantissa variable is already greater than 0.
Vulnerability Detail
The current implementation of the smart contract disallows the operator from setting the feeMantissa variable to a value greater than 0 if the feeRecipient address is set to address(0). However, it still allows the operator to set the feeRecipient address to the zero address if the feeMantissa variable is already greater than 0.
It will lead to "dead" shares added to totalSupply in each created pool.
Impact
The getCurrentState() function is called in each major function of the Pool contract and has a check feeMantissa variable that is greater than 0.
As a result, the check in a function that ensures the feeMantissa is not set to the zero address will pass, and the function will calculate new _accruedFeeShares and add them to thetotalSupply and balanceOf[address(0)].
This behavior will result in dead shares that cannot be withdrawn and will impact the entire Pool logic.
Smart contract should be modified to disallow the operator from setting the feeRecipient to the zero address if the feeMantissa variable is set to a value greater than 0.
slvDev
high
feeRecipient can be set to address(0) if the feeMantissa variable is greater than 0
Summary
An operator is able (accidentally or on purpose) to set the feeRecipient address to the zero address if the feeMantissa variable is already greater than 0.
Vulnerability Detail
The current implementation of the smart contract disallows the operator from setting the feeMantissa variable to a value greater than 0 if the feeRecipient address is set to address(0). However, it still allows the operator to set the feeRecipient address to the zero address if the feeMantissa variable is already greater than 0. It will lead to "dead" shares added to totalSupply in each created pool.
Impact
The
getCurrentState()function is called in each major function of the Pool contract and has a check feeMantissa variable that is greater than 0. As a result, the check in a function that ensures the feeMantissa is not set to the zero address will pass, and the function will calculate new_accruedFeeSharesand add them to thetotalSupplyandbalanceOf[address(0)]. This behavior will result indeadshares that cannot be withdrawn and will impact the entire Pool logic.Code Snippet
https://github.com/sherlock-audit/2023-02-surge/blob/main/surge-protocol-v1/src/Factory.sol#L49-L65 https://github.com/sherlock-audit/2023-02-surge/blob/main/surge-protocol-v1/src/Pool.sol#L93-L166
Tool used
Manual Review
Recommendation
Smart contract should be modified to disallow the operator from setting the feeRecipient to the zero address if the feeMantissa variable is set to a value greater than 0.
Duplicate of #124