From 7f4f655cca91bc44b32e2cd0907e980a352230d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Fri, 25 Mar 2022 20:21:06 +0100 Subject: [PATCH] Disallow RETURNDATASIZE and RETURNDATACOPY in inline assembly blocks in pure functions (cherry picked from commit f567eb1fb2a24dbd956fe65512d4b9015ba7f5ed) quick fix to pass failing test at ./test/externalTests/ens.sh (line 80) -- Should be removed when 0.9 is released. --- Changelog.md | 1 + libevmasm/SemanticInformation.cpp | 2 + test/externalTests/ens.sh | 3 ++ .../inlineAssembly/evm_byzantium.sol | 2 +- .../evm_byzantium_on_homestead.sol | 2 +- ...ze_as_variable_call_post_byzantium.sol.sol | 2 +- ...urndatasize_as_variable_post_byzantium.sol | 2 +- ...turndatasize_as_variable_pre_byzantium.sol | 2 +- ...ine_assembly_instructions_allowed_pure.sol | 10 ++-- ..._assembly_instructions_disallowed_pure.sol | 52 ++++++++++--------- 10 files changed, 44 insertions(+), 34 deletions(-) diff --git a/Changelog.md b/Changelog.md index 9cf8098d9..1f223a5d7 100644 --- a/Changelog.md +++ b/Changelog.md @@ -8,6 +8,7 @@ Breaking changes: * Commandline Interface: Remapping targets are not automatically added to allowed paths. * Commandline Interface: Assembler mode no longer enables all outputs by default. * General: The identifier ``basefee`` is a reserved identifier in Yul for all EVM versions. + * View Pure Checker: Mark ``returndatasize`` and ``returndatacopy`` as view to disallow them in inline assembly blocks in pure functions. ### 0.8.15 (unreleased) diff --git a/libevmasm/SemanticInformation.cpp b/libevmasm/SemanticInformation.cpp index 16c78541c..1a3b3e5da 100644 --- a/libevmasm/SemanticInformation.cpp +++ b/libevmasm/SemanticInformation.cpp @@ -481,6 +481,8 @@ bool SemanticInformation::invalidInPureFunctions(Instruction _instruction) case Instruction::EXTCODESIZE: case Instruction::EXTCODECOPY: case Instruction::EXTCODEHASH: + case Instruction::RETURNDATASIZE: + case Instruction::RETURNDATACOPY: case Instruction::BLOCKHASH: case Instruction::COINBASE: case Instruction::TIMESTAMP: diff --git a/test/externalTests/ens.sh b/test/externalTests/ens.sh index 2b177afb2..8d17920b2 100755 --- a/test/externalTests/ens.sh +++ b/test/externalTests/ens.sh @@ -75,6 +75,9 @@ function ens_test # In some cases Hardhat does not detect revert reasons properly via IR. # TODO: Remove this when https://github.com/NomicFoundation/hardhat/issues/2115 gets fixed. sed -i "s|it\(('Does not allow wrapping a name you do not own',\)|it.skip\1|g" test/wrapper/NameWrapper.js + # Related to disallow returndatasize and returndatacopy in inline assembly blocks in pure functions https://github.com/ethereum/solidity/pull/13028 + # TODO: Remove this after release 0.9 and ENS updated to support it. + sed -i 's/pure/view/' contracts/utils/LowLevelCallUtils.sol find . -name "*.sol" -exec sed -i -e 's/^\(\s*\)\(assembly\)/\1\/\/\/ @solidity memory-safe-assembly\n\1\2/' '{}' \; diff --git a/test/libsolidity/syntaxTests/inlineAssembly/evm_byzantium.sol b/test/libsolidity/syntaxTests/inlineAssembly/evm_byzantium.sol index b08172ebe..28d336577 100644 --- a/test/libsolidity/syntaxTests/inlineAssembly/evm_byzantium.sol +++ b/test/libsolidity/syntaxTests/inlineAssembly/evm_byzantium.sol @@ -1,5 +1,5 @@ contract C { - function f() pure external { + function f() view external { assembly { let s := returndatasize() returndatacopy(0, 0, s) diff --git a/test/libsolidity/syntaxTests/inlineAssembly/evm_byzantium_on_homestead.sol b/test/libsolidity/syntaxTests/inlineAssembly/evm_byzantium_on_homestead.sol index 74316c526..5698f990f 100644 --- a/test/libsolidity/syntaxTests/inlineAssembly/evm_byzantium_on_homestead.sol +++ b/test/libsolidity/syntaxTests/inlineAssembly/evm_byzantium_on_homestead.sol @@ -1,5 +1,5 @@ contract C { - function f() pure external { + function f() view external { assembly { let s := returndatasize() returndatacopy(0, 0, s) diff --git a/test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_call_post_byzantium.sol.sol b/test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_call_post_byzantium.sol.sol index f959537b6..403022846 100644 --- a/test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_call_post_byzantium.sol.sol +++ b/test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_call_post_byzantium.sol.sol @@ -1,5 +1,5 @@ contract C { - function f() public pure { + function f() public view { uint returndatasize; returndatasize; assembly { diff --git a/test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_post_byzantium.sol b/test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_post_byzantium.sol index 44a12aa06..903da7223 100644 --- a/test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_post_byzantium.sol +++ b/test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_post_byzantium.sol @@ -1,5 +1,5 @@ contract C { - function f() public pure { + function f() public view { uint returndatasize; returndatasize; assembly { diff --git a/test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_pre_byzantium.sol b/test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_pre_byzantium.sol index 07bae8ac8..250a0755f 100644 --- a/test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_pre_byzantium.sol +++ b/test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_pre_byzantium.sol @@ -1,4 +1,4 @@ -contract C { function f() public pure { uint returndatasize; returndatasize; assembly { pop(returndatasize()) }}} +contract C { function f() public view { uint returndatasize; returndatasize; assembly { pop(returndatasize()) }}} // ==== // EVMVersion: =homestead // ---- diff --git a/test/libsolidity/syntaxTests/viewPureChecker/inline_assembly_instructions_allowed_pure.sol b/test/libsolidity/syntaxTests/viewPureChecker/inline_assembly_instructions_allowed_pure.sol index 53c91c74f..ab41323a0 100644 --- a/test/libsolidity/syntaxTests/viewPureChecker/inline_assembly_instructions_allowed_pure.sol +++ b/test/libsolidity/syntaxTests/viewPureChecker/inline_assembly_instructions_allowed_pure.sol @@ -47,8 +47,8 @@ contract C { //codecopy(0, 1, 2) //pop(extcodesize(0)) //extcodecopy(0, 1, 2, 3) - pop(returndatasize()) - returndatacopy(0, 1, 2) + //pop(returndatasize()) + //returndatacopy(0, 1, 2) //pop(extcodehash(0)) //pop(create(0, 1, 2)) //pop(create2(0, 1, 2, 3)) @@ -85,6 +85,6 @@ contract C { // ==== // EVMVersion: >=london // ---- -// Warning 5740: (94-1759): Unreachable code. -// Warning 5740: (1772-1784): Unreachable code. -// Warning 5740: (1827-1836): Unreachable code. +// Warning 5740: (94-1763): Unreachable code. +// Warning 5740: (1776-1788): Unreachable code. +// Warning 5740: (1831-1840): Unreachable code. diff --git a/test/libsolidity/syntaxTests/viewPureChecker/inline_assembly_instructions_disallowed_pure.sol b/test/libsolidity/syntaxTests/viewPureChecker/inline_assembly_instructions_disallowed_pure.sol index f9a652fdb..8da17390d 100644 --- a/test/libsolidity/syntaxTests/viewPureChecker/inline_assembly_instructions_disallowed_pure.sol +++ b/test/libsolidity/syntaxTests/viewPureChecker/inline_assembly_instructions_disallowed_pure.sol @@ -11,6 +11,8 @@ contract C { pop(callvalue()) pop(extcodesize(0)) extcodecopy(0, 1, 2, 3) + pop(returndatasize()) + returndatacopy(0, 1, 2) pop(extcodehash(0)) pop(create(0, 1, 2)) pop(create2(0, 1, 2, 3)) @@ -44,7 +46,7 @@ contract C { // ==== // EVMVersion: >=london // ---- -// Warning 5740: (672-1083): Unreachable code. +// Warning 5740: (742-1153): Unreachable code. // TypeError 2527: (79-87): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". // TypeError 8961: (101-113): Function cannot be declared as pure because this expression (potentially) modifies the state. // TypeError 2527: (130-135): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". @@ -55,26 +57,28 @@ contract C { // TypeError 2527: (265-276): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". // TypeError 2527: (294-308): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". // TypeError 2527: (322-345): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". -// TypeError 2527: (362-376): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". -// TypeError 8961: (394-409): Function cannot be declared as pure because this expression (potentially) modifies the state. -// TypeError 8961: (427-446): Function cannot be declared as pure because this expression (potentially) modifies the state. -// TypeError 8961: (464-489): Function cannot be declared as pure because this expression (potentially) modifies the state. -// TypeError 8961: (507-536): Function cannot be declared as pure because this expression (potentially) modifies the state. -// TypeError 8961: (554-584): Function cannot be declared as pure because this expression (potentially) modifies the state. -// TypeError 2527: (602-630): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". -// TypeError 8961: (644-659): Function cannot be declared as pure because this expression (potentially) modifies the state. -// TypeError 8961: (672-682): Function cannot be declared as pure because this expression (potentially) modifies the state. -// TypeError 8961: (695-708): Function cannot be declared as pure because this expression (potentially) modifies the state. -// TypeError 8961: (721-737): Function cannot be declared as pure because this expression (potentially) modifies the state. -// TypeError 8961: (750-769): Function cannot be declared as pure because this expression (potentially) modifies the state. -// TypeError 8961: (782-804): Function cannot be declared as pure because this expression (potentially) modifies the state. -// TypeError 2527: (821-830): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". -// TypeError 2527: (848-857): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". -// TypeError 2527: (875-883): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". -// TypeError 2527: (901-911): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". -// TypeError 2527: (929-941): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". -// TypeError 2527: (959-969): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". -// TypeError 2527: (987-998): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". -// TypeError 2527: (1016-1024): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". -// TypeError 2527: (1042-1054): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". -// TypeError 2527: (1072-1082): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". +// TypeError 2527: (362-378): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". +// TypeError 2527: (392-415): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". +// TypeError 2527: (432-446): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". +// TypeError 8961: (464-479): Function cannot be declared as pure because this expression (potentially) modifies the state. +// TypeError 8961: (497-516): Function cannot be declared as pure because this expression (potentially) modifies the state. +// TypeError 8961: (534-559): Function cannot be declared as pure because this expression (potentially) modifies the state. +// TypeError 8961: (577-606): Function cannot be declared as pure because this expression (potentially) modifies the state. +// TypeError 8961: (624-654): Function cannot be declared as pure because this expression (potentially) modifies the state. +// TypeError 2527: (672-700): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". +// TypeError 8961: (714-729): Function cannot be declared as pure because this expression (potentially) modifies the state. +// TypeError 8961: (742-752): Function cannot be declared as pure because this expression (potentially) modifies the state. +// TypeError 8961: (765-778): Function cannot be declared as pure because this expression (potentially) modifies the state. +// TypeError 8961: (791-807): Function cannot be declared as pure because this expression (potentially) modifies the state. +// TypeError 8961: (820-839): Function cannot be declared as pure because this expression (potentially) modifies the state. +// TypeError 8961: (852-874): Function cannot be declared as pure because this expression (potentially) modifies the state. +// TypeError 2527: (891-900): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". +// TypeError 2527: (918-927): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". +// TypeError 2527: (945-953): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". +// TypeError 2527: (971-981): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". +// TypeError 2527: (999-1011): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". +// TypeError 2527: (1029-1039): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". +// TypeError 2527: (1057-1068): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". +// TypeError 2527: (1086-1094): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". +// TypeError 2527: (1112-1124): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view". +// TypeError 2527: (1142-1152): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view".