Address cs audit 5.5 (#123)

* address cs audit 5.5 * now empty code is not valid * Update test/quark-core/CodeJar.t.sol Co-authored-by: Kevin Cheng <[email protected]> * Update test/quark-core/CodeJar.t.sol Co-authored-by: Kevin Cheng <[email protected]> * address comments, remove line 43, and use saveCode() * update gas snapshot * update snapshot --------- Co-authored-by: Kevin Cheng <[email protected]>
compound-finance · Dec 13, 2023 · 6791726 · 6791726
1 parent 3e46e98
commit 6791726
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 18 deletions.
diff --git a/.gas-snapshot b/.gas-snapshot
@@ -8,15 +8,16 @@ CallbacksTest:testCallcodeReentrancyProtection() (gas: 189755)
 CallbacksTest:testNestedCallWithNoCallbackSucceeds() (gas: 118558)
 CallbacksTest:testPayableCallback() (gas: 114193)
 CallbacksTest:testRevertsOnCallbackWhenNoActiveCallback() (gas: 100390)
+CodeJarTest:testCodeJarCodeDoesNotExistOnEmptyScriptWithETH() (gas: 56347)
 CodeJarTest:testCodeJarCounter() (gas: 377506)
-CodeJarTest:testCodeJarDeployConstructor() (gas: 47037)
-CodeJarTest:testCodeJarDeployNotAffectedByChangedCodeHash() (gas: 81705)
-CodeJarTest:testCodeJarDifferentZeros() (gas: 90061)
+CodeJarTest:testCodeJarDeployConstructor() (gas: 47059)
+CodeJarTest:testCodeJarDeployNotAffectedByChangedCodeHash() (gas: 81727)
+CodeJarTest:testCodeJarDifferentZeros() (gas: 90083)
 CodeJarTest:testCodeJarFirstDeploy() (gas: 44694)
-CodeJarTest:testCodeJarInputVariety() (gas: 451689)
-CodeJarTest:testCodeJarLarge() (gas: 70676086)
-CodeJarTest:testCodeJarSecondDeploy() (gas: 47605)
-CodeJarTest:testCodeJarSelfDestruct() (gas: 50952)
+CodeJarTest:testCodeJarInputVariety() (gas: 407557)
+CodeJarTest:testCodeJarLarge() (gas: 70676108)
+CodeJarTest:testCodeJarSecondDeploy() (gas: 47627)
+CodeJarTest:testCodeJarSelfDestruct() (gas: 50976)
 CometClaimRewardsTest:testClaimComp() (gas: 132589)
 CometRepayAndWithdrawMultipleAssetsTest:testInvalidInput() (gas: 78255)
 CometRepayAndWithdrawMultipleAssetsTest:testRepayAndWithdrawMultipleAssets() (gas: 161976)

diff --git a/quark-core/src/CodeJar.sol b/quark-core/src/CodeJar.sol
@@ -40,15 +40,14 @@ contract CodeJar {
 
     /**
      * @notice Checks if code was already deployed by CodeJar
-     * @dev Use `saveCode` to get the address of the contract with that code
      * @param code The runtime bytecode of the code to check
      * @return True if code already exists in Code Jar
      */
     function codeExists(bytes calldata code) external view returns (bool) {
         bytes memory initCode = getInitCode(code);
         address codeAddress = getCodeAddress(initCode);
 
-        return codeAddress.codehash == keccak256(code);
+        return codeAddress.code.length != 0 && codeAddress.codehash == keccak256(code);
     }
 
     /**

diff --git a/test/quark-core/CodeJar.t.sol b/test/quark-core/CodeJar.t.sol
@@ -84,16 +84,15 @@ contract CodeJarTest is Test {
     }
 
     function testCodeJarInputVariety() public {
-        bytes[] memory scripts = new bytes[](8);
-        scripts[0] = hex"";
-        scripts[1] = hex"00";
-        scripts[2] = hex"11";
-        scripts[3] = hex"112233";
-        scripts[4] = hex"00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff";
-        scripts[5] = hex"00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff11";
-        scripts[6] =
+        bytes[] memory scripts = new bytes[](7);
+        scripts[0] = hex"00";
+        scripts[1] = hex"11";
+        scripts[2] = hex"112233";
+        scripts[3] = hex"00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff";
+        scripts[4] = hex"00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff11";
+        scripts[5] =
             hex"00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff";
-        scripts[7] =
+        scripts[6] =
             hex"00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff11";
 
         for (uint8 i = 0; i < scripts.length; i++) {
@@ -155,5 +154,16 @@ contract CodeJarTest is Test {
         assertEq(returnData, hex"abcd");
     }
 
+    function testCodeJarCodeDoesNotExistOnEmptyScriptWithETH() public {
+        bytes memory code = hex"";
+        assertEq(codeJar.codeExists(code), false);
+        address scriptAddress = codeJar.saveCode(code);
+        vm.deal(address(this), 1 ether);
+        scriptAddress.call{value: 1}("");
+
+        // Ensure codeExists correctness holds for empty code with ETH
+        assertEq(codeJar.codeExists(code), false);
+    }
+
     // Note: cannot test code too large, as overflow impossible to test
 }