💚 add validation in setContest and getProxyAddress (#34)

L-11. Insufficient validation leads to locking up prize tokens forever #12
codefox-inc · Sep 20, 2023 · 7364c77 · 7364c77
1 parent 24d0623
commit 7364c77
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 10 deletions.
diff --git a/src/ProxyFactory.sol b/src/ProxyFactory.sol
@@ -50,6 +50,7 @@ contract ProxyFactory is Ownable, EIP712 {
     error ProxyFactory__ContestIsNotRegistered();
     error ProxyFactory__ContestIsNotExpired();
     error ProxyFactory__ProxyAddressCannotBeZero();
+    error ProxyFactory__ImplementationNotDeployed();
     error ProxyFactory__ProxyAddressMismatch();
     error ProxyFactory__DelegateCallFailed();
     error ProxyFactory__ProxyIsNotAContract();
@@ -112,6 +113,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();
         }
@@ -229,6 +231,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)));

diff --git a/test/fuzz/FuzzTestProxyFactory.t.sol b/test/fuzz/FuzzTestProxyFactory.t.sol
@@ -99,16 +99,17 @@ contract FuzzTestProxyFactory is StdCheats, HelperContract {
         vm.stopPrank();
     }
 
-    function testFuzzCanSetContestWithAnyImplementation(address randomImple) public {
-        vm.assume(randomImple != address(0));
-        bytes32 randomId = keccak256(abi.encode("Jim", "001"));
-        vm.startPrank(factoryAdmin);
-        uint256 inputTime = block.timestamp + 1 days;
-        proxyFactory.setContest(organizer, randomId, inputTime, randomImple);
-        vm.stopPrank();
-        bytes32 salt_ = keccak256(abi.encode(organizer, randomId, randomImple));
-        assertTrue(proxyFactory.saltToCloseTime(salt_) == inputTime);
-    }
+    // due to adding the check of implementation's code size, this fuzz test is not usable anymore
+    // function testFuzzCanSetContestWithAnyImplementation(address randomImple) public {
+    //     vm.assume(randomImple != address(0));
+    //     bytes32 randomId = keccak256(abi.encode("Jim", "001"));
+    //     vm.startPrank(factoryAdmin);
+    //     uint256 inputTime = block.timestamp + 1 days;
+    //     proxyFactory.setContest(organizer, randomId, inputTime, randomImple);
+    //     vm.stopPrank();
+    //     bytes32 salt_ = keccak256(abi.encode(organizer, randomId, randomImple));
+    //     assertTrue(proxyFactory.saltToCloseTime(salt_) == inputTime);
+    // }
 
     function testFuzzCanSetContestWithAnyId(bytes32 randomId) public {
         vm.startPrank(factoryAdmin);