cyfrin / 2023-09-sparkn-mitigation2 Goto Github PK

    @@ -107,6 +115,7 @@ contract ProxyFactory is Ownable, EIP712 {
         onlyOwner
     {
         if (organizer == address(0) || implementation == address(0)) revert ProxyFactory__NoZeroAddress();
+        if (implementation.code.length == 0) revert ProxyFactory__ImplementationNotDeployed();
         if (closeTime > block.timestamp + MAX_CONTEST_PERIOD || closeTime < block.timestamp) {
             revert ProxyFactory__CloseTimeNotInRange();
         }

diff --git a/src/ProxyFactory.sol b/src/ProxyFactory.sol
index d7a1960..0bd8575 100644
--- a/src/ProxyFactory.sol
+++ b/src/ProxyFactory.sol
@@ -77,6 +77,7 @@ contract ProxyFactory is Ownable, EIP712 {
     mapping(bytes32 => uint256) public saltToCloseTime;
     /// @dev record whitelisted tokens
     mapping(address => bool) public whitelistedTokens;
+    mapping(address => uint256) public implementations;
 
     ////////////////////////////
     /////// Constructor ////////
@@ -100,6 +101,9 @@ contract ProxyFactory is Ownable, EIP712 {
     ////////////////////////////////////////////
     /////// External & Public functions ////////
     ////////////////////////////////////////////
+    function addImplementation(address implementation) external onlyOwner {
+        implementations[implementation] = 1;
+    }
     /**
      * @notice Only owner can set contest's properties
      * @notice close time must be less than 28 days from now
@@ -110,12 +114,13 @@ contract ProxyFactory is Ownable, EIP712 {
      * @param closeTime The contest close time
      * @param implementation The implementation address
      */
+
     function setContest(address organizer, bytes32 contestId, uint256 closeTime, address implementation)
         public
         onlyOwner
     {
-        if (organizer == address(0) || implementation == address(0)) revert ProxyFactory__NoZeroAddress();
-        if (implementation.code.length == 0) revert ProxyFactory__ImplementationNotDeployed();
+        if (organizer == address(0)) revert ProxyFactory__NoZeroAddress();
+        if (implementations[implementation] == 0) revert ProxyFactory__ImplementationNotDeployed();
         if (closeTime > block.timestamp + MAX_CONTEST_PERIOD || closeTime < block.timestamp) {
             revert ProxyFactory__CloseTimeNotInRange();
         }

 @@ -223,6 +233,7 @@ contract ProxyFactory is Ownable, EIP712 {
     /// @param implementation The implementation address
     /// @return proxy The calculated proxy address
     function getProxyAddress(bytes32 salt, address implementation) public view returns (address proxy) {
+        if (saltToCloseTime[salt] == 0) revert ProxyFactory__ContestIsNotRegistered();
         bytes memory code = abi.encodePacked(type(Proxy).creationCode, uint256(uint160(implementation)));
         bytes32 hash = keccak256(abi.encodePacked(bytes1(0xff), address(this), salt, keccak256(code)));
         proxy = address(uint160(uint256(hash)));

@@ -239,6 +244,15 @@ contract ProxyFactory is Ownable, EIP712 {
         proxy = address(uint160(uint256(hash)));
     }
 
+    function getProxyAddressSafely(address organizer, bytes32 contestId, address implementation)
+        external
+        view
+        returns (address proxy)
+    {
+        bytes32 salt = _calculateSalt(organizer, contestId, implementation);
+        proxy = getProxyAddress(salt, implementation);
+    }
+

diff --git a/src/ProxyFactory.sol b/src/ProxyFactory.sol
index d7a1960..c77559a 100644
--- a/src/ProxyFactory.sol
+++ b/src/ProxyFactory.sol
@@ -165,8 +165,9 @@ contract ProxyFactory is Ownable, EIP712 {
         bytes calldata signature,
         bytes calldata data
     ) public returns (address) {
-        bytes32 digest =
-            _hashTypedDataV4(keccak256(abi.encode(_DEPLOY_AND_DISTRIBUTE_TYPEHASH, contestId, implementation, data)));
+        bytes32 digest = _hashTypedDataV4(
+            keccak256(abi.encode(_DEPLOY_AND_DISTRIBUTE_TYPEHASH, contestId, implementation, keccak256(data)))
+        );
         if (!organizer.isValidSignatureNow(digest, signature)) revert ProxyFactory__InvalidSignature();
         bytes32 salt = _calculateSalt(organizer, contestId, implementation);
         if (saltToCloseTime[salt] == 0) revert ProxyFactory__ContestIsNotRegistered();

...

    event SetContest(
        address indexed organizer, bytes32 indexed contestId, uint256 closeTime, address indexed implementation
    );
+   event SetStadiumAddress(address indexed stadiumAddress);
    event Distributed(address indexed proxy, bytes data);

...

    // contest distribution expiration
    bytes32 internal constant _DEPLOY_AND_DISTRIBUTE_TYPEHASH =
        keccak256("DeployAndDistribute(bytes32 contestId,address implementation,bytes data)");
    uint256 public constant EXPIRATION_TIME = 7 days;
    uint256 public constant MAX_CONTEST_PERIOD = 28 days;
+   address public _stadiumAddress;

...

+   /**
+    * @notice Only owner can set stadium address
+    * @dev Set stadium address
+    * @param _newStadiumAddress The new stadium address
+    */
+   function setStadiumAddress(address _newStadiumAddress) public onlyOwner {
+       _stadiumAddress = _newStadiumAddress;
+       emit SetStadiumAddress(_newStadiumAddress);
+   }

...

...

    uint8 private constant VERSION = 1; // version is 1 for now
    address private immutable FACTORY_ADDRESS;
-   address private immutable STADIUM_ADDRESS;
    uint256 private constant COMMISSION_FEE = 500; // this can be changed in the future
    // a constant value of 10,000 (basis points) = 100%
    uint256 private constant BASIS_POINTS = 10000;

...

    /// @dev initiate the contract with factory address and other key addresses, fee rate
    constructor(
        // uint256 version, // for future use
-       address factoryAddress,
-       address stadiumAddress
+       address factoryAddress
    ) 
    /* solhint-enable */
    {
-      if (factoryAddress == address(0) || stadiumAddress == address(0)) revert Distributor__NoZeroAddress();
+      if (factoryAddress == address(0)) revert Distributor__NoZeroAddress();
        FACTORY_ADDRESS = factoryAddress; // initialize with deployed factory address beforehand
-       STADIUM_ADDRESS = stadiumAddress; // official address to receive commission fee
    }
...
    function _commissionTransfer(IERC20 token) internal {
-      token.safeTransfer(STADIUM_ADDRESS, token.balanceOf(address(this)));
+      token.safeTransfer(getStadiumAddress(), token.balanceOf(address(this)));
    }

...
    /**
     * @notice returns all the immutable and constant addresses and values
     * @dev This function is for convenience to check the addresses and values
     */
    function getConstants()
        external
        view
        returns (address _FACTORY_ADDRESS, address _STADIUM_ADDRESS, uint256 _COMMISSION_FEE, uint8 _VERSION)
    {
        /* solhint-disable */
        _FACTORY_ADDRESS = FACTORY_ADDRESS;
-     _STADIUM_ADDRESS = STADIUM_ADDRESS;
+     _STADIUM_ADDRESS = getStadiumAddress();
        _COMMISSION_FEE = COMMISSION_FEE;
        _VERSION = VERSION;
        /* solhint-enable */
    }
+
+    /**
+    * @notice returns stadium address from proxy factory
+    * @dev This function is for convenience to get the stadium address
+    */
+   function getStadiumAddress() internal view returns (address) {
+       return ProxyFactory(FACTORY_ADDRESS)._stadiumAddress();
+   }

    function distributeByOwner(
-       address proxy,
        address organizer,
        bytes32 contestId,
        address implementation,
        bytes calldata data
    ) public onlyOwner {
-       if (proxy == address(0)) revert ProxyFactory__ProxyAddressCannotBeZero();
        bytes32 salt = _calculateSalt(organizer, contestId, implementation);
-       if (proxy != getProxyAddress(salt, implementation)) {
-           revert ProxyFactory__ProxyAddressMismatch();
-       }
        // distribute only when it exists and expired
        if (saltToCloseTime[salt] + EXPIRATION_TIME > block.timestamp) revert ProxyFactory__ContestIsNotExpired();
+       address proxy = getProxyAddress(salt, implementation);     
        _distribute(proxy, data);
    }

diff --git a/src/Distributor.sol b/src/Distributor.sol
index 876af3b..a0f2742 100644
--- a/src/Distributor.sol
+++ b/src/Distributor.sol
@@ -88,7 +88,7 @@ contract Distributor {
      * @param winners The addresses array of winners
      * @param percentages The percentages array of winners
      */
-    function distribute(address token, address[] memory winners, uint256[] memory percentages, bytes memory data)
+    function distribute(address token, address[] calldata winners, uint256[] calldata percentages, bytes calldata data)
         external
     {
         if (msg.sender != FACTORY_ADDRESS) {
@@ -112,7 +112,7 @@ contract Distributor {
      * @param percentages The percentages of winners
      * @param data The data to be logged. It is supposed to be used for showing the realation bbetween winners and proposals.
      */
-    function _distribute(address token, address[] memory winners, uint256[] memory percentages, bytes memory data)
+    function _distribute(address token, address[] calldata winners, uint256[] calldata percentages, bytes calldata data)
         internal
     {
         // token address input check
@@ -121,7 +121,8 @@ contract Distributor {
             revert Distributor__InvalidTokenAddress();
         }
         // winners and percentages input check
-        if (winners.length == 0 || winners.length != percentages.length) revert Distributor__MismatchedArrays();
+        uint256 winnersLength = winners.length; // cache length
+        if (winnersLength == 0 || winnersLength != percentages.length) revert Distributor__MismatchedArrays();
 
         // prepare for the loop
         IERC20 erc20 = IERC20(token);
@@ -129,15 +130,19 @@ contract Distributor {
         uint256 totalAmount = erc20.balanceOf(address(this));
         // if there is no token to distribute, then revert
         if (totalAmount == 0) revert Distributor__NoTokenToDistribute();
-        
+
         // percentages.length = winners length
-        uint256 winnersLength = winners.length; // cache length
+
         uint256 totalPercentage;
         for (uint256 i; i < winnersLength;) {
-            totalPercentage += percentages[i];
-            uint256 amount = totalAmount * percentages[i] / BASIS_POINTS;
-            if (winners[i] == address(0)) revert Distributor__NoZeroAddress();
-            erc20.safeTransfer(winners[i], amount);
+            uint256 percentage = percentages[i];
+
+            totalPercentage += percentage;
+            uint256 amount = totalAmount * percentage / BASIS_POINTS;
+
+            address winner = winners[i];
+            if (winner == address(0)) revert Distributor__NoZeroAddress();
+            erc20.safeTransfer(winner, amount);
             unchecked {
                 ++i;
             }

diff --git a/src/ProxyFactory.sol b/src/ProxyFactory.sol
index d7a1960..8156679 100644
--- a/src/ProxyFactory.sol
+++ b/src/ProxyFactory.sol
@@ -204,26 +204,22 @@ contract ProxyFactory is Ownable, EIP712 {
      * @notice Owner can rescue funds if token is stuck after the deployment and contest is over for a while
      * @dev only owner can call this function and it is supposed not to be called often
      * @dev fee sent to stadium address is included in the logic contract
-     * @param proxy The proxy address
      * @param organizer The contest organizer
      * @param contestId The contest id
      * @param implementation The implementation address
      * @param data The prize distribution calling data
      */
     function distributeByOwner(
-        address proxy,
+        // address proxy,
         address organizer,
         bytes32 contestId,
         address implementation,
         bytes calldata data
     ) public onlyOwner {
-        if (proxy == address(0)) revert ProxyFactory__ProxyAddressCannotBeZero();
         bytes32 salt = _calculateSalt(organizer, contestId, implementation);
-        if (proxy != getProxyAddress(salt, implementation)) {
-            revert ProxyFactory__ProxyAddressMismatch();
-        }
         // distribute only when it exists and expired
         if (saltToCloseTime[salt] + EXPIRATION_TIME > block.timestamp) revert ProxyFactory__ContestIsNotExpired();
+        address proxy = getProxyAddress(salt, implementation);
         _distribute(proxy, data);
     }

diff --git a/src/ProxyFactory.sol b/src/ProxyFactory.sol
index d7a1960..7662cd8 100644
--- a/src/ProxyFactory.sol
+++ b/src/ProxyFactory.sol
@@ -224,6 +224,7 @@ contract ProxyFactory is Ownable, EIP712 {
         }
         // distribute only when it exists and expired
         if (saltToCloseTime[salt] + EXPIRATION_TIME > block.timestamp) revert ProxyFactory__ContestIsNotExpired();
+        if (proxy.code.length == 0) revert ProxyFactory__ProxyIsNotAContract();
         _distribute(proxy, data);
     }
 
@@ -259,7 +260,6 @@ contract ProxyFactory is Ownable, EIP712 {
     /// @param proxy The proxy address
     /// @param data The prize distribution data
     function _distribute(address proxy, bytes calldata data) internal {
-        if (proxy.code.length == 0) revert ProxyFactory__ProxyIsNotAContract();
         (bool success,) = proxy.call(data);
         if (!success) revert ProxyFactory__DelegateCallFailed();
         emit Distributed(proxy, data);

    function deployProxyAndDistributeBySignature(
        address organizer,
        bytes32 contestId,
        address implementation,
        bytes calldata signature,
        bytes calldata data
    ) public returns (address) {
        bytes32 digest =
-           _hashTypedDataV4(keccak256(abi.encode(_DEPLOY_AND_DISTRIBUTE_TYPEHASH, contestId, implementation, data)));
+           _hashTypedDataV4(
+               keccak256(
+                   abi.encode(
+                       _DEPLOY_AND_DISTRIBUTE_TYPEHASH, 
+                       contestId, 
+                       implementation, 
+                       keccak256(data)
+                   )
+               )
+           );
        if (!organizer.isValidSignatureNow(digest, signature)) revert ProxyFactory__InvalidSignature();
        bytes32 salt = _calculateSalt(organizer, contestId, implementation);
        if (saltToCloseTime[salt] == 0) revert ProxyFactory__ContestIsNotRegistered();
        if (saltToCloseTime[salt] > block.timestamp) revert ProxyFactory__ContestIsNotClosed();
        address proxy = _deployProxy(organizer, contestId, implementation);
        _distribute(proxy, data);
        return proxy;
    }