diff --git a/Changelog.md b/Changelog.md index 8e3c57c9d..3ef7ac7b9 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,8 @@ ### 0.8.3 (unreleased) +Important Bugfixes: + * Optimizer: Fix bug on incorrect caching of Keccak-256 hashes. + Language Features: diff --git a/docs/bugs.json b/docs/bugs.json index 0f3be018a..ddcd69aa0 100644 --- a/docs/bugs.json +++ b/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.", diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index 2ffa87e41..a3a851744 100644 --- a/docs/bugs_by_version.json +++ b/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" } } \ No newline at end of file diff --git a/test/libsolidity/semanticTests/inlineAssembly/keccak256_optimization.sol b/test/libsolidity/semanticTests/inlineAssembly/keccak256_optimization.sol new file mode 100644 index 000000000..59b32a4c9 --- /dev/null +++ b/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 diff --git a/test/libsolidity/semanticTests/inlineAssembly/keccak256_optimizer_bug_different_memory_location.sol b/test/libsolidity/semanticTests/inlineAssembly/keccak256_optimizer_bug_different_memory_location.sol new file mode 100644 index 000000000..1e9847be3 --- /dev/null +++ b/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 diff --git a/test/libsolidity/semanticTests/inlineAssembly/keccak256_optimizer_cache_bug.sol b/test/libsolidity/semanticTests/inlineAssembly/keccak256_optimizer_cache_bug.sol new file mode 100644 index 000000000..545ad3617 --- /dev/null +++ b/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 diff --git a/test/libsolidity/semanticTests/inlineAssembly/keccak_optimization_bug_string.sol b/test/libsolidity/semanticTests/inlineAssembly/keccak_optimization_bug_string.sol new file mode 100644 index 000000000..009b514e0 --- /dev/null +++ b/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