Sparkn

Summary

The factoryAddress and the stadiumAddress can be same in the constructor of the Distributor.sol

Vulnerability Details

There is no check for the factoryAddress and the stadiumAddress to be different.

Instances include:

https://github.com/Cyfrin/2023-08-sparkn/blob/main/src/Distributor.sol#L76C5-L81C1

constructor(
    // uint256 version, // for future use
    address factoryAddress,
    address stadiumAddress
) 
/* solhint-enable */
{
    if (factoryAddress == address(0) || stadiumAddress == address(0)) revert Distributor__NoZeroAddress();
    FACTORY_ADDRESS = factoryAddress; // initialize with deployed factory address beforehand
    STADIUM_ADDRESS = stadiumAddress; // official address to receive commission fee
}

Impact

Having the same address for factoryAddress and stadiumAddress can lead to the commission fee of the protocol going to the factoryAddress and the funds getting locked as there is no way to get the funds back. This will happen when the the same factoryAddress is used in the place of stadiumAddress also.

Tools Used

Manual review and VS Code

Recommendations

Add the following custom error in the constructor of the Distributor.sol :

if (factoryAddress == stadiumAddress ) revert Distributor__NoSameAddress();

Note: Also create a new custom error for ex. Distributor__NoSameAddress();

constructor(
    // uint256 version, // for future use
    address factoryAddress,
    address stadiumAddress
) 
/* solhint-enable */
{
    if (factoryAddress == address(0) || stadiumAddress == address(0)) revert Distributor__NoZeroAddress();
    if (factoryAddress == stadiumAddress ) revert Distributor__NoSameAddress();
    FACTORY_ADDRESS = factoryAddress; // initialize with deployed factory address beforehand
    STADIUM_ADDRESS = stadiumAddress; // official address to receive commission fee
}