Changelog and test for the KeccakCaching bug

Changelog.md

@@ -1,5 +1,8 @@
 ### 0.8.3 (unreleased)
 
+Important Bugfixes:
+ * Optimizer: Fix bug on incorrect caching of Keccak-256 hashes.
+
 Language Features:
 
 

docs/bugs.json

@@ -1,4 +1,15 @@
 [
+    {
+        "name": "KeccakCaching",
+        "summary": "The bytecode optimizer incorrectly re-used previously evaluated Keccak-256 hashes. You are unlikely to be affected if you do not compute Keccak-256 hashes in inline assembly.",
+        "description": "Solidity's bytecode optimizer has a step that can compute Keccak-256 hashes, if the contents of the memory are known during compilation time. This step also has a mechanism to determine that two Keccak-256 hashes are equal even if the values in memory are not known during compile time. This mechanism had a bug where Keccak-256 of the same memory content, but different sizes were considered equal. More specifically, ``keccak256(mpos1, length1)`` and ``keccak256(mpos2, length2)`` in some cases were considered equal if ``length1`` and ``length2``, when rounded up to nearest multiple of 32 were the same, and when the memory contents at ``mpos1`` and ``mpos2`` can be deduced to be equal. You maybe affected if you compute multiple Keccak-256 hashes of the same content, but with different lengths inside inline assembly. You are unaffected if your code uses ``keccak256`` with a length that is not a compile-time constant or if it is always a multiple of 32.",
+        "link": "https://blog.soliditylang.org/2021/03/23/keccak-optimizer-bug/",
+        "fixed": "0.8.3",
+        "conditions": {
+            "optimizer": true
+        },
+        "severity": "medium"
+    },
     {
         "name": "EmptyByteArrayCopy",
         "summary": "Copying an empty byte array (or string) from memory or calldata to storage can result in data corruption if the target array's length is increased subsequently without storing new data.",

docs/bugs_by_version.json

@@ -1,6 +1,7 @@
 {
     "0.1.0": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ExpExponentCleanup",
@@ -21,6 +22,7 @@
     },
     "0.1.1": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ExpExponentCleanup",
@@ -41,6 +43,7 @@
     },
     "0.1.2": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ExpExponentCleanup",
@@ -61,6 +64,7 @@
     },
     "0.1.3": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ExpExponentCleanup",
@@ -81,6 +85,7 @@
     },
     "0.1.4": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ExpExponentCleanup",
@@ -102,6 +107,7 @@
     },
     "0.1.5": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ExpExponentCleanup",
@@ -123,6 +129,7 @@
     },
     "0.1.6": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -146,6 +153,7 @@
     },
     "0.1.7": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -169,6 +177,7 @@
     },
     "0.2.0": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -193,6 +202,7 @@
     },
     "0.2.1": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -217,6 +227,7 @@
     },
     "0.2.2": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -241,6 +252,7 @@
     },
     "0.3.0": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -267,6 +279,7 @@
     },
     "0.3.1": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -292,6 +305,7 @@
     },
     "0.3.2": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -317,6 +331,7 @@
     },
     "0.3.3": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -341,6 +356,7 @@
     },
     "0.3.4": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -365,6 +381,7 @@
     },
     "0.3.5": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -389,6 +406,7 @@
     },
     "0.3.6": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -411,6 +429,7 @@
     },
     "0.4.0": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -433,6 +452,7 @@
     },
     "0.4.1": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -455,6 +475,7 @@
     },
     "0.4.10": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -476,6 +497,7 @@
     },
     "0.4.11": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -496,6 +518,7 @@
     },
     "0.4.12": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -515,6 +538,7 @@
     },
     "0.4.13": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -534,6 +558,7 @@
     },
     "0.4.14": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -552,6 +577,7 @@
     },
     "0.4.15": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -569,6 +595,7 @@
     },
     "0.4.16": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -588,6 +615,7 @@
     },
     "0.4.17": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -608,6 +636,7 @@
     },
     "0.4.18": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -627,6 +656,7 @@
     },
     "0.4.19": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -647,6 +677,7 @@
     },
     "0.4.2": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -668,6 +699,7 @@
     },
     "0.4.20": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -688,6 +720,7 @@
     },
     "0.4.21": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -708,6 +741,7 @@
     },
     "0.4.22": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -728,6 +762,7 @@
     },
     "0.4.23": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -747,6 +782,7 @@
     },
     "0.4.24": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -766,6 +802,7 @@
     },
     "0.4.25": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -783,6 +820,7 @@
     },
     "0.4.26": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -797,6 +835,7 @@
     },
     "0.4.3": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -817,6 +856,7 @@
     },
     "0.4.4": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "TupleAssignmentMultiStackSlotComponents",
@@ -836,6 +876,7 @@
     },
     "0.4.5": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -858,6 +899,7 @@
     },
     "0.4.6": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -879,6 +921,7 @@
     },
     "0.4.7": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -900,6 +943,7 @@
     },
     "0.4.8": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -921,6 +965,7 @@
     },
     "0.4.9": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -942,6 +987,7 @@
     },
     "0.5.0": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -959,6 +1005,7 @@
     },
     "0.5.1": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -976,6 +1023,7 @@
     },
     "0.5.10": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -989,6 +1037,7 @@
     },
     "0.5.11": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -1001,6 +1050,7 @@
     },
     "0.5.12": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -1013,6 +1063,7 @@
     },
     "0.5.13": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -1025,6 +1076,7 @@
     },
     "0.5.14": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "MissingEscapingInFormatting",
@@ -1039,6 +1091,7 @@
     },
     "0.5.15": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "MissingEscapingInFormatting",
@@ -1052,6 +1105,7 @@
     },
     "0.5.16": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "MissingEscapingInFormatting",
@@ -1064,6 +1118,7 @@
     },
     "0.5.17": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "MissingEscapingInFormatting",
@@ -1075,6 +1130,7 @@
     },
     "0.5.2": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -1092,6 +1148,7 @@
     },
     "0.5.3": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -1109,6 +1166,7 @@
     },
     "0.5.4": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -1126,6 +1184,7 @@
     },
     "0.5.5": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -1145,6 +1204,7 @@
     },
     "0.5.6": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -1164,6 +1224,7 @@
     },
     "0.5.7": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -1181,6 +1242,7 @@
     },
     "0.5.8": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -1197,6 +1259,7 @@
     },
     "0.5.9": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "ImplicitConstructorCallvalueCheck",
@@ -1212,6 +1275,7 @@
     },
     "0.6.0": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "MissingEscapingInFormatting",
@@ -1225,6 +1289,7 @@
     },
     "0.6.1": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "MissingEscapingInFormatting",
@@ -1237,6 +1302,7 @@
     },
     "0.6.10": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup"
         ],
@@ -1244,6 +1310,7 @@
     },
     "0.6.11": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup"
         ],
@@ -1251,6 +1318,7 @@
     },
     "0.6.12": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup"
         ],
@@ -1258,6 +1326,7 @@
     },
     "0.6.2": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "MissingEscapingInFormatting",
@@ -1270,6 +1339,7 @@
     },
     "0.6.3": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "MissingEscapingInFormatting",
@@ -1282,6 +1352,7 @@
     },
     "0.6.4": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "MissingEscapingInFormatting",
@@ -1294,6 +1365,7 @@
     },
     "0.6.5": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "MissingEscapingInFormatting",
@@ -1305,6 +1377,7 @@
     },
     "0.6.6": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "MissingEscapingInFormatting",
@@ -1315,6 +1388,7 @@
     },
     "0.6.7": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "MissingEscapingInFormatting",
@@ -1325,6 +1399,7 @@
     },
     "0.6.8": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup"
         ],
@@ -1332,6 +1407,7 @@
     },
     "0.6.9": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "UsingForCalldata"
@@ -1340,6 +1416,7 @@
     },
     "0.7.0": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup"
         ],
@@ -1347,6 +1424,7 @@
     },
     "0.7.1": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup",
             "FreeFunctionRedefinition"
@@ -1355,6 +1433,7 @@
     },
     "0.7.2": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy",
             "DynamicArrayCleanup"
         ],
@@ -1362,32 +1441,45 @@
     },
     "0.7.3": {
         "bugs": [
+            "KeccakCaching",
             "EmptyByteArrayCopy"
         ],
         "released": "2020-10-07"
     },
     "0.7.4": {
-        "bugs": [],
+        "bugs": [
+            "KeccakCaching"
+        ],
         "released": "2020-10-19"
     },
     "0.7.5": {
-        "bugs": [],
+        "bugs": [
+            "KeccakCaching"
+        ],
         "released": "2020-11-18"
     },
     "0.7.6": {
-        "bugs": [],
+        "bugs": [
+            "KeccakCaching"
+        ],
         "released": "2020-12-16"
     },
     "0.8.0": {
-        "bugs": [],
+        "bugs": [
+            "KeccakCaching"
+        ],
         "released": "2020-12-16"
     },
     "0.8.1": {
-        "bugs": [],
+        "bugs": [
+            "KeccakCaching"
+        ],
         "released": "2021-01-27"
     },
     "0.8.2": {
-        "bugs": [],
+        "bugs": [
+            "KeccakCaching"
+        ],
         "released": "2021-03-02"
     }
 }

test/libsolidity/semanticTests/inlineAssembly/keccak256_optimization.sol

@@ -0,0 +1,16 @@
+contract C {
+    function f() public view returns (bool ret) {
+        assembly {
+            let x := calldataload(0)
+            mstore(0, x)
+            mstore(0x20, x)
+            let a := keccak256(0, 4)
+            let b := keccak256(0x20, 4)
+            ret := eq(a, b)
+        }
+    }
+}
+// ====
+// compileViaYul: also
+// ----
+// f() -> true

test/libsolidity/semanticTests/inlineAssembly/keccak256_optimizer_bug_different_memory_location.sol

@@ -0,0 +1,16 @@
+contract C {
+    function f() public view returns (bool ret) {
+        assembly {
+            let x := calldataload(0)
+            mstore(0, x)
+            mstore(0x20, x)
+            let a := keccak256(0, 4)
+            let b := keccak256(0x20, 8)
+            ret := eq(a, b)
+        }
+    }
+}
+// ====
+// compileViaYul: also
+// ----
+// f() -> false

test/libsolidity/semanticTests/inlineAssembly/keccak256_optimizer_cache_bug.sol

@@ -0,0 +1,21 @@
+contract C {
+  uint[] data;
+
+  function val() public returns (bool) {
+    assembly {
+        sstore(0, 2)
+        mstore(0, 0)
+        sstore(keccak256(0, 32), 234)
+        // A bug in the caching mechanism previously caused keccak256(0, 23) to be the same as
+        // keccak256(0, 32), leading to `data[1] == 123` being true.
+        sstore(add(keccak256(0, 23), 1), 123)
+    }
+    assert(data[1] != 123);
+    assert(data[1] == 0);
+    return true;
+  }
+}
+// ====
+// compileViaYul: also
+// ----
+// val() -> true

test/libsolidity/semanticTests/inlineAssembly/keccak_optimization_bug_string.sol

@@ -0,0 +1,15 @@
+contract C {
+    function f(string memory s) public returns (bool ret) {
+        assembly {
+            let a := keccak256(s, 32)
+            let b := keccak256(s, 8)
+            ret := eq(a, b)
+        }
+    }
+}
+// ====
+// compileViaYul: also
+// ----
+// f(string): "" -> false
+// f(string): 0x20, 5, "hello" -> false
+// f(string): 0x20, 0x2e, 29457663690442756349866640336617293820574110049925353194191585327958485180523, 45859201465615193776739262511799714667061496775486067316261261194408342061056 -> false