From e9f1bd00ccecfc359d903d7d6e995eece89bba05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Mon, 16 May 2022 16:59:32 +0200 Subject: [PATCH 01/12] gnosis: Workaround for problems caused by forcing ethers@5.6.1 in external tests --- test/externalTests/gnosis.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/externalTests/gnosis.sh b/test/externalTests/gnosis.sh index 4ad66479a..bbf35cae0 100755 --- a/test/externalTests/gnosis.sh +++ b/test/externalTests/gnosis.sh @@ -85,6 +85,10 @@ function gnosis_safe_test # TODO: Remove when https://github.com/ethers-io/ethers.js/discussions/2849 is resolved. npm install ethers@5.6.1 + # Note that ethers@5.6.1 depends on @ethersproject/contracts@5.6.0 while the dependency on hardhat-deploy + # pulls @ethersproject/contracts@5.6.1 (latest). Force 5.6.0 to avoid errors due to having two copies. + npm install @ethersproject/contracts@5.6.0 + # Hardhat 2.9.5 introduced a bug with handling padded arguments to getStorageAt(). # TODO: Remove when https://github.com/NomicFoundation/hardhat/issues/2709 is fixed. npm install hardhat@2.9.4 From 91177d74eea8b643042b32b47bac0c802736d3eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Mon, 16 May 2022 20:52:53 +0200 Subject: [PATCH 02/12] perpetual-pools: Remove the ethers@5.6.1 workaround --- test/externalTests/perpetual-pools.sh | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/externalTests/perpetual-pools.sh b/test/externalTests/perpetual-pools.sh index 1b54fc577..8f90ed0b4 100755 --- a/test/externalTests/perpetual-pools.sh +++ b/test/externalTests/perpetual-pools.sh @@ -66,10 +66,6 @@ function perpetual_pools_test force_hardhat_unlimited_contract_size "$config_file" "$config_var" yarn install - # With ethers.js 5.6.2 many tests for revert messages fail. - # TODO: Remove when https://github.com/ethers-io/ethers.js/discussions/2849 is resolved. - yarn add ethers@5.6.1 - replace_version_pragmas for preset in $SELECTED_PRESETS; do From f021017bc000e55eefd1ee18495e073878759837 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 17 May 2022 10:38:26 +0200 Subject: [PATCH 03/12] Sort changelog. --- Changelog.md | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/Changelog.md b/Changelog.md index 5ffb45f78..51686bc74 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,25 +1,22 @@ ### 0.8.14 (unreleased) Important Bugfixes: - * ABI Encoding: When ABI-encoding values from calldata that contain nested arrays, correctly validate the nested array length against ``calldatasize()`` in all cases. - - -Language Features: + * ABI Encoder: When ABI-encoding values from calldata that contain nested arrays, correctly validate the nested array length against ``calldatasize()`` in all cases. Compiler Features: - * Peephole Optimizer: Remove operations without side effects before simple terminations. - * Assembly-Json: Export: Include source list in `sourceList` field. - * Commandline Interface: option ``--pretty-json`` works also with the following options: ``--abi``, ``--asm-json``, ``--ast-compact-json``, ``--devdoc``, ``--storage-layout``, ``--userdoc``. - * SMTChecker: Support ``abi.encodeCall`` taking into account the called selector. + * Assembly-Json Exporter: Include source list in `sourceList` field. + * Commandline Interface: Option ``--pretty-json`` works also with the following options: ``--abi``, ``--asm-json``, ``--ast-compact-json``, ``--devdoc``, ``--storage-layout``, ``--userdoc``. * Language Server: Allow full filesystem access to language server. + * Peephole Optimizer: Remove operations without side effects before simple terminations. + * SMTChecker: Support ``abi.encodeCall`` taking into account the called selector. Bugfixes: - * Type Checker: Properly check restrictions of ``using ... global`` in conjunction with libraries. - * Assembly-Json: Fix assembly json export to store jump types of operations in `jumpType` field instead of `value`. - * SMTChecker: Fix bug when z3 is selected but not available at runtime. + * Assembly-Json Exporter: Fix assembly json export to store jump types of operations in `jumpType` field instead of `value`. * SMTChecker: Fix ABI compatibility with z3 >=4.8.16. + * SMTChecker: Fix bug when z3 is selected but not available at runtime. + * Type Checker: Properly check restrictions of ``using ... global`` in conjunction with libraries. * TypeChecker: Convert parameters of function type to how they would be called for ``abi.encodeCall``. From c57bc4706047915ed98288bc8dc42ee9a3d267bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Mon, 16 May 2022 22:43:50 +0200 Subject: [PATCH 04/12] gnosis: Disable newly added tests broken due to Hardhat heuristics --- test/externalTests/gnosis.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/externalTests/gnosis.sh b/test/externalTests/gnosis.sh index bbf35cae0..5c02baa16 100755 --- a/test/externalTests/gnosis.sh +++ b/test/externalTests/gnosis.sh @@ -73,6 +73,11 @@ function gnosis_safe_test # them for other presets but that's fine - we want same code run for benchmarks to be comparable. # TODO: Remove this when Hardhat adjusts heuristics for IR (https://github.com/nomiclabs/hardhat/issues/2115). sed -i "s|\(it\)\(('should not allow to call setup on singleton'\)|\1.skip\2|g" test/core/GnosisSafe.Setup.spec.ts + # TODO: Remove this when https://github.com/NomicFoundation/hardhat/issues/2453 gets fixed. + sed -i 's|\(it\)\(("changes the expected storage slot without touching the most important ones"\)|\1.skip\2|g' test/libraries/SignMessageLib.spec.ts + sed -i "s|\(it\)\(('can be used only via DELEGATECALL opcode'\)|\1.skip\2|g" test/libraries/SignMessageLib.spec.ts + sed -i 's|\(describe\)\(("Upgrade from Safe 1.1.1"\)|\1.skip\2|g' test/migration/UpgradeFromSafe111.spec.ts + sed -i 's|\(describe\)\(("Upgrade from Safe 1.2.0"\)|\1.skip\2|g' test/migration/UpgradeFromSafe120.spec.ts neutralize_package_lock neutralize_package_json_hooks From adf3eaac9bf0282cb2efd09861a66ba03c6d661c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Mon, 16 May 2022 22:43:31 +0200 Subject: [PATCH 05/12] gnosis: Update upstream repo URL --- test/externalTests/gnosis.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/externalTests/gnosis.sh b/test/externalTests/gnosis.sh index 5c02baa16..7d579f76c 100755 --- a/test/externalTests/gnosis.sh +++ b/test/externalTests/gnosis.sh @@ -36,7 +36,7 @@ function test_fn { npm test; } function gnosis_safe_test { - local repo="https://github.com/gnosis/safe-contracts.git" + local repo="https://github.com/safe-global/safe-contracts.git" local ref_type=branch local ref=main local config_file="hardhat.config.ts" From dfa0bcf760ddbebed102be227d906ebb066562f3 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 24 Mar 2022 15:03:02 +0100 Subject: [PATCH 06/12] More strict override check for data locations. --- libsolidity/analysis/ContractLevelChecker.cpp | 4 +- libsolidity/analysis/OverrideChecker.cpp | 52 +++++++++++++++++-- libsolidity/analysis/OverrideChecker.h | 6 ++- libsolidity/ast/AST.cpp | 9 +++- .../dataLocation/external_public_calldata.sol | 18 +++++++ .../external_overriding_external.sol | 14 +++++ ..._data_location_change_illegal_internal.sol | 13 +++++ ..._change_calldata_memory_illegal_public.sol | 11 ++++ ..._data_location_change_illegal_internal.sol | 16 ++++++ ...er_data_location_change_illegal_public.sol | 16 ++++++ .../return_type_data_location.sol | 7 +++ ...turn_type_data_location_change_illegal.sol | 8 +++ tools/solidityUpgrade/Upgrade060.cpp | 2 +- 13 files changed, 165 insertions(+), 11 deletions(-) create mode 100644 test/libsolidity/semanticTests/inheritance/dataLocation/external_public_calldata.sol create mode 100644 test/libsolidity/syntaxTests/inheritance/dataLocation/external_overriding_external.sol create mode 100644 test/libsolidity/syntaxTests/inheritance/dataLocation/modifier_parameter_data_location_change_illegal_internal.sol create mode 100644 test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_calldata_memory_illegal_public.sol create mode 100644 test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_illegal_internal.sol create mode 100644 test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_illegal_public.sol create mode 100644 test/libsolidity/syntaxTests/inheritance/dataLocation/return_type_data_location.sol create mode 100644 test/libsolidity/syntaxTests/inheritance/dataLocation/return_type_data_location_change_illegal.sol diff --git a/libsolidity/analysis/ContractLevelChecker.cpp b/libsolidity/analysis/ContractLevelChecker.cpp index c159b8357..9ecf76f13 100644 --- a/libsolidity/analysis/ContractLevelChecker.cpp +++ b/libsolidity/analysis/ContractLevelChecker.cpp @@ -39,7 +39,7 @@ namespace { template -bool hasEqualParameters(T const& _a, B const& _b) +bool hasEqualExternalCallableParameters(T const& _a, B const& _b) { return FunctionType(_a).asExternallyCallableFunction(false)->hasEqualParameterTypes( *FunctionType(_b).asExternallyCallableFunction(false) @@ -204,7 +204,7 @@ void ContractLevelChecker::findDuplicateDefinitions(map> const SecondarySourceLocation ssl; for (size_t j = i + 1; j < overloads.size(); ++j) - if (hasEqualParameters(*overloads[i], *overloads[j])) + if (hasEqualExternalCallableParameters(*overloads[i], *overloads[j])) { solAssert( ( diff --git a/libsolidity/analysis/OverrideChecker.cpp b/libsolidity/analysis/OverrideChecker.cpp index 2f83748fb..d342ab100 100644 --- a/libsolidity/analysis/OverrideChecker.cpp +++ b/libsolidity/analysis/OverrideChecker.cpp @@ -313,7 +313,7 @@ Token OverrideProxy::functionKind() const }, m_item); } -FunctionType const* OverrideProxy::functionType() const +FunctionType const* OverrideProxy::externalFunctionType() const { return std::visit(GenericVisitor{ [&](FunctionDefinition const* _item) { return FunctionType(*_item).asExternallyCallableFunction(false); }, @@ -322,6 +322,15 @@ FunctionType const* OverrideProxy::functionType() const }, m_item); } +FunctionType const* OverrideProxy::originalFunctionType() const +{ + return std::visit(GenericVisitor{ + [&](FunctionDefinition const* _item) { return TypeProvider::function(*_item); }, + [&](VariableDeclaration const*) -> FunctionType const* { solAssert(false, "Requested specific function type of variable."); return nullptr; }, + [&](ModifierDefinition const*) -> FunctionType const* { solAssert(false, "Requested specific function type of modifier."); return nullptr; } + }, m_item); +} + ModifierType const* OverrideProxy::modifierType() const { return std::visit(GenericVisitor{ @@ -413,7 +422,7 @@ OverrideProxy::OverrideComparator const& OverrideProxy::overrideComparator() con [&](FunctionDefinition const* _function) { vector paramTypes; - for (Type const* t: functionType()->parameterTypes()) + for (Type const* t: externalFunctionType()->parameterTypes()) paramTypes.emplace_back(t->richIdentifier()); return OverrideComparator{ _function->name(), @@ -424,7 +433,7 @@ OverrideProxy::OverrideComparator const& OverrideProxy::overrideComparator() con [&](VariableDeclaration const* _var) { vector paramTypes; - for (Type const* t: functionType()->parameterTypes()) + for (Type const* t: externalFunctionType()->parameterTypes()) paramTypes.emplace_back(t->richIdentifier()); return OverrideComparator{ _var->name(), @@ -589,14 +598,17 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr if (_super.isFunction()) { - FunctionType const* functionType = _overriding.functionType(); - FunctionType const* superType = _super.functionType(); + FunctionType const* functionType = _overriding.externalFunctionType(); + FunctionType const* superType = _super.externalFunctionType(); + bool returnTypesDifferAlready = false; if (_overriding.functionKind() != Token::Fallback) { solAssert(functionType->hasEqualParameterTypes(*superType), "Override doesn't have equal parameters!"); if (!functionType->hasEqualReturnTypes(*superType)) + { + returnTypesDifferAlready = true; overrideError( _overriding, _super, @@ -604,6 +616,36 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr "Overriding " + _overriding.astNodeName() + " return types differ.", "Overridden " + _overriding.astNodeName() + " is here:" ); + } + } + + // The override proxy considers calldata and memory the same data location. + // Here we do a more specific check: + // Data locations of parameters and return variables have to match + // unless we have a public function overriding an external one. + if ( + _overriding.isFunction() && + !returnTypesDifferAlready && + _super.visibility() != Visibility::External && + _overriding.functionKind() != Token::Fallback + ) + { + if (!_overriding.originalFunctionType()->hasEqualParameterTypes(*_super.originalFunctionType())) + overrideError( + _overriding, + _super, + 7723_error, + "Data locations of parameters have to be the same when overriding non-external functions, but they differ.", + "Overridden " + _overriding.astNodeName() + " is here:" + ); + if (!_overriding.originalFunctionType()->hasEqualReturnTypes(*_super.originalFunctionType())) + overrideError( + _overriding, + _super, + 1443_error, + "Data locations of return variables have to be the same when overriding non-external functions, but they differ.", + "Overridden " + _overriding.astNodeName() + " is here:" + ); } // Stricter mutability is always okay except when super is Payable diff --git a/libsolidity/analysis/OverrideChecker.h b/libsolidity/analysis/OverrideChecker.h index 9178d179d..824248325 100644 --- a/libsolidity/analysis/OverrideChecker.h +++ b/libsolidity/analysis/OverrideChecker.h @@ -84,7 +84,10 @@ public: /// @returns receive / fallback / function (only the latter for modifiers and variables); langutil::Token functionKind() const; - FunctionType const* functionType() const; + /// @returns the externally callable function type + FunctionType const* externalFunctionType() const; + /// @returns the (unmodified) function type + FunctionType const* originalFunctionType() const; ModifierType const* modifierType() const; Declaration const* declaration() const; @@ -101,6 +104,7 @@ public: /** * Struct to help comparing override items about whether they override each other. + * Compares functions based on their "externally callable" type. * Does not produce a total order. */ struct OverrideComparator diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp index be6f2fe9b..04c47a155 100644 --- a/libsolidity/ast/AST.cpp +++ b/libsolidity/ast/AST.cpp @@ -480,7 +480,9 @@ FunctionDefinition const& FunctionDefinition::resolveVirtual( solAssert(isOrdinary(), ""); solAssert(!libraryFunction(), ""); - FunctionType const* functionType = TypeProvider::function(*this)->asExternallyCallableFunction(false); + // We actually do not want the externally callable function here. + // This is just to add an assertion since the comparison used to be less strict. + FunctionType const* externalFunctionType = TypeProvider::function(*this)->asExternallyCallableFunction(false); bool foundSearchStart = (_searchStart == nullptr); for (ContractDefinition const* c: _mostDerivedContract.annotation().linearizedBaseContracts) @@ -495,9 +497,12 @@ FunctionDefinition const& FunctionDefinition::resolveVirtual( // With super lookup analysis guarantees that there is an implemented function in the chain. // With virtual lookup there are valid cases where returning an unimplemented one is fine. (function->isImplemented() || _searchStart == nullptr) && - FunctionType(*function).asExternallyCallableFunction(false)->hasEqualParameterTypes(*functionType) + FunctionType(*function).asExternallyCallableFunction(false)->hasEqualParameterTypes(*externalFunctionType) ) + { + solAssert(FunctionType(*function).hasEqualParameterTypes(*TypeProvider::function(*this))); return *function; + } } solAssert(false, "Virtual function " + name() + " not found."); diff --git a/test/libsolidity/semanticTests/inheritance/dataLocation/external_public_calldata.sol b/test/libsolidity/semanticTests/inheritance/dataLocation/external_public_calldata.sol new file mode 100644 index 000000000..4de95b0f1 --- /dev/null +++ b/test/libsolidity/semanticTests/inheritance/dataLocation/external_public_calldata.sol @@ -0,0 +1,18 @@ +abstract contract A { + function f(uint256[] calldata a) external virtual returns (uint256[] calldata); +} + +contract B is A { + function f(uint256[] memory a) public override returns (uint256[] memory) { + return a; + } + + function g(uint[] calldata x) public returns (uint256[] memory) { + return f(x); + } +} +// ==== +// compileViaYul: also +// ---- +// f(uint256[]): 0x20, 2, 9, 8 -> 0x20, 2, 9, 8 +// g(uint256[]): 0x20, 2, 9, 8 -> 0x20, 2, 9, 8 diff --git a/test/libsolidity/syntaxTests/inheritance/dataLocation/external_overriding_external.sol b/test/libsolidity/syntaxTests/inheritance/dataLocation/external_overriding_external.sol new file mode 100644 index 000000000..6105a50a2 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/dataLocation/external_overriding_external.sol @@ -0,0 +1,14 @@ +abstract contract A { + function f(uint256[1] memory a) external virtual returns (uint256); +} +contract B is A { + function f(uint256[1] calldata a) external pure virtual override returns (uint256) { + return a[0]; + } +} +contract C is A, B { + function f(uint256[1] memory a) external pure override(B, A) returns (uint256) { + return a[0]; + } +} +// ---- diff --git a/test/libsolidity/syntaxTests/inheritance/dataLocation/modifier_parameter_data_location_change_illegal_internal.sol b/test/libsolidity/syntaxTests/inheritance/dataLocation/modifier_parameter_data_location_change_illegal_internal.sol new file mode 100644 index 000000000..a4856d9f0 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/dataLocation/modifier_parameter_data_location_change_illegal_internal.sol @@ -0,0 +1,13 @@ +abstract contract A { + modifier m(uint256[1] memory a) virtual; + function test(uint256[1] memory a) m(a) external { + } +} + +contract B is A { + modifier m(uint256[1] calldata a) override { + _; + } +} +// ---- +// TypeError 1078: (153-214): Override changes modifier signature. diff --git a/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_calldata_memory_illegal_public.sol b/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_calldata_memory_illegal_public.sol new file mode 100644 index 000000000..e3fb069af --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_calldata_memory_illegal_public.sol @@ -0,0 +1,11 @@ +abstract contract A { + function f(uint256[1] calldata a) public virtual returns (uint256); +} + +contract B is A { + function f(uint256[1] memory a) public override returns (uint256) { + return a[0]; + } +} +// ---- +// TypeError 7723: (119-213): Data locations of parameters have to be the same when overriding non-external functions, but they differ. diff --git a/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_illegal_internal.sol b/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_illegal_internal.sol new file mode 100644 index 000000000..55b5b5fe6 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_illegal_internal.sol @@ -0,0 +1,16 @@ +abstract contract A { + function f(uint256[1] memory a) internal virtual returns (uint256); + function test() external returns (uint) { + uint[1] memory t; + t[0] = 7; + return f(t); + } +} + +contract B is A { + function f(uint256[1] calldata a) internal override returns (uint256) { + return a[0]; + } +} +// ---- +// TypeError 7723: (236-334): Data locations of parameters have to be the same when overriding non-external functions, but they differ. diff --git a/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_illegal_public.sol b/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_illegal_public.sol new file mode 100644 index 000000000..52123f68d --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/dataLocation/parameter_data_location_change_illegal_public.sol @@ -0,0 +1,16 @@ +abstract contract A { + function f(uint256[1] memory a) public virtual returns (uint256); + function test() external returns (uint) { + uint[1] memory t; + t[0] = 7; + return f(t); + } +} + +contract B is A { + function f(uint256[1] calldata a) public override returns (uint256) { + return a[0]; + } +} +// ---- +// TypeError 7723: (234-330): Data locations of parameters have to be the same when overriding non-external functions, but they differ. diff --git a/test/libsolidity/syntaxTests/inheritance/dataLocation/return_type_data_location.sol b/test/libsolidity/syntaxTests/inheritance/dataLocation/return_type_data_location.sol new file mode 100644 index 000000000..1c7f4e926 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/dataLocation/return_type_data_location.sol @@ -0,0 +1,7 @@ +abstract contract A { + function foo() external virtual view returns(uint[] calldata); +} +contract X is A { + function foo() public view override returns(uint[] memory) { } +} +// ---- diff --git a/test/libsolidity/syntaxTests/inheritance/dataLocation/return_type_data_location_change_illegal.sol b/test/libsolidity/syntaxTests/inheritance/dataLocation/return_type_data_location_change_illegal.sol new file mode 100644 index 000000000..e06cd43ae --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/dataLocation/return_type_data_location_change_illegal.sol @@ -0,0 +1,8 @@ +abstract contract A { + function foo() public virtual view returns(uint[] calldata); +} +contract X is A { + function foo() public view override returns(uint[] memory) { } +} +// ---- +// TypeError 1443: (105-168): Data locations of return variables have to be the same when overriding non-external functions, but they differ. diff --git a/tools/solidityUpgrade/Upgrade060.cpp b/tools/solidityUpgrade/Upgrade060.cpp index 52010db39..8caff02c9 100644 --- a/tools/solidityUpgrade/Upgrade060.cpp +++ b/tools/solidityUpgrade/Upgrade060.cpp @@ -72,7 +72,7 @@ void OverridingFunction::endVisit(ContractDefinition const& _contract) { auto& super = (*begin); auto functionType = FunctionType(*function).asExternallyCallableFunction(false); - auto superType = super.functionType()->asExternallyCallableFunction(false); + auto superType = super.externalFunctionType(); if (functionType && functionType->hasEqualParameterTypes(*superType)) { From 27e5afa23d1c9f52b7cb36906f665d9c1f2c0f78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Thu, 24 Mar 2022 20:00:00 +0100 Subject: [PATCH 07/12] Patch external tests for the override data alignment issue until our patches are accepted upstream --- test/externalTests/bleeps.sh | 8 ++++++++ test/externalTests/brink.sh | 3 +++ test/externalTests/gnosis.sh | 3 +++ 3 files changed, 14 insertions(+) diff --git a/test/externalTests/bleeps.sh b/test/externalTests/bleeps.sh index bd3b0255b..a9d2b58a0 100755 --- a/test/externalTests/bleeps.sh +++ b/test/externalTests/bleeps.sh @@ -86,6 +86,14 @@ function bleeps_test npm install npm-run-all npm install + # TODO: Bleeps depends on OpenZeppelin 4.3.2, which is affected by + # https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3293. + # Forcing OZ >= 4.6.0 fixes this but it also causes a lot of unrelated compilation errors. + # Remove this when Bleeps gets updated to support newer OpenZeppelin. + perl -i -0pe \ + "s/(function hashProposal\(\n address\[\] )calldata( targets,\n uint256\[\] )calldata( values,\n bytes\[\] )calldata( calldatas,)/\1memory\2memory\3memory\4/g" \ + node_modules/@openzeppelin/contracts/governance/IGovernor.sol + replace_version_pragmas for preset in $SELECTED_PRESETS; do diff --git a/test/externalTests/brink.sh b/test/externalTests/brink.sh index 1513350bb..bfbd993f2 100755 --- a/test/externalTests/brink.sh +++ b/test/externalTests/brink.sh @@ -62,6 +62,9 @@ function brink_test setup_solc "$DIR" "$BINARY_TYPE" "$BINARY_PATH" download_project "$repo" "$ref_type" "$ref" "$DIR" + # TODO: Remove this when Brink merges https://github.com/brinktrade/brink-core/pull/52 + sed -i "s|\(function isValidSignature(bytes \)calldata\( _data, bytes \)calldata\( _signature)\)|\1memory\2memory\3|g" contracts/Test/MockEIP1271Validator.sol + neutralize_package_lock neutralize_package_json_hooks force_hardhat_compiler_binary "$config_file" "$BINARY_TYPE" "$BINARY_PATH" diff --git a/test/externalTests/gnosis.sh b/test/externalTests/gnosis.sh index 7d579f76c..8995231e1 100755 --- a/test/externalTests/gnosis.sh +++ b/test/externalTests/gnosis.sh @@ -79,6 +79,9 @@ function gnosis_safe_test sed -i 's|\(describe\)\(("Upgrade from Safe 1.1.1"\)|\1.skip\2|g' test/migration/UpgradeFromSafe111.spec.ts sed -i 's|\(describe\)\(("Upgrade from Safe 1.2.0"\)|\1.skip\2|g' test/migration/UpgradeFromSafe120.spec.ts + # TODO: Remove this when Gnosis merges https://github.com/gnosis/safe-contracts/pull/394 + sed -i "s|\(function isValidSignature(bytes \)calldata\( _data, bytes \)calldata\( _signature)\)|\1memory\2memory\3|g" contracts/handler/CompatibilityFallbackHandler.sol + neutralize_package_lock neutralize_package_json_hooks force_hardhat_compiler_binary "$config_file" "$BINARY_TYPE" "$BINARY_PATH" From 1164d1b4dd1ab808ebd7149d50b983cdf59834b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Thu, 7 Apr 2022 21:07:48 +0200 Subject: [PATCH 08/12] Switch ENS external test to master branch --- test/externalTests/ens.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/externalTests/ens.sh b/test/externalTests/ens.sh index bf018d0ce..2b177afb2 100755 --- a/test/externalTests/ens.sh +++ b/test/externalTests/ens.sh @@ -37,8 +37,8 @@ function test_fn { yarn test; } function ens_test { local repo="https://github.com/ensdomains/ens-contracts.git" - local ref_type=tag - local ref="v0.0.8" # The project is in flux right now and master might be too unstable for us + local ref_type=branch + local ref="master" local config_file="hardhat.config.js" local compile_only_presets=( @@ -72,6 +72,10 @@ function ens_test replace_version_pragmas neutralize_packaged_contracts + # 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 + find . -name "*.sol" -exec sed -i -e 's/^\(\s*\)\(assembly\)/\1\/\/\/ @solidity memory-safe-assembly\n\1\2/' '{}' \; for preset in $SELECTED_PRESETS; do From 259edd4c98e10e31f580ba28c9ef6e14e4416ba7 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 2 May 2022 12:27:20 +0200 Subject: [PATCH 09/12] Changelog. --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 51686bc74..ebaa17f24 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,7 @@ Important Bugfixes: * ABI Encoder: When ABI-encoding values from calldata that contain nested arrays, correctly validate the nested array length against ``calldatasize()`` in all cases. + * Override Checker: Allow changing data location for parameters only when overriding external functions. Compiler Features: From f427247993c795b86ec66414b018ef3b9499cf21 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 2 May 2022 13:30:28 +0200 Subject: [PATCH 10/12] Bug list entry. --- docs/bugs.json | 11 +++++++++++ docs/bugs_by_version.json | 25 +++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/docs/bugs.json b/docs/bugs.json index 6613f1964..7bee4d10f 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -1,4 +1,15 @@ [ + { + "uid": "SOL-2022-3", + "name": "DataLocationChangeInInternalOverride", + "summary": "It was possible to change the data location of the parameters or return variables from ``calldata`` to ``memory`` and vice-versa while overriding internal and public functions. This caused invalid code to be generated when calling such a function internally through virtual function calls.", + "description": "When calling external functions, it is irrelevant if the data location of the parameters is ``calldata`` or ``memory``, the encoding of the data does not change. Because of that, changing the data location when overriding external functions is allowed. The compiler incorrectly also allowed a change in the data location for overriding public and internal functions. Since public functions can be called internally as well as externally, this causes invalid code to be generated when such an incorrectly overridden function is called internally through the base contract. The caller provides a memory pointer, but the called function interprets it as a calldata pointer or vice-versa.", + "link": "https://blog.soliditylang.org/2022/05/17/data-location-inheritance-bug/", + "introduced": "0.6.9", + "fixed": "0.8.14", + "severity": "very low" + + }, { "uid": "SOL-2022-2", "name": "NestedCallataArrayAbiReencodingSizeValidation", diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index 0d103279f..d6ee8bb8a 100644 --- a/docs/bugs_by_version.json +++ b/docs/bugs_by_version.json @@ -1345,6 +1345,7 @@ }, "0.6.10": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1356,6 +1357,7 @@ }, "0.6.11": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1367,6 +1369,7 @@ }, "0.6.12": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1477,6 +1480,7 @@ }, "0.6.9": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1489,6 +1493,7 @@ }, "0.7.0": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1500,6 +1505,7 @@ }, "0.7.1": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1512,6 +1518,7 @@ }, "0.7.2": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1523,6 +1530,7 @@ }, "0.7.3": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1533,6 +1541,7 @@ }, "0.7.4": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1542,6 +1551,7 @@ }, "0.7.5": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1551,6 +1561,7 @@ }, "0.7.6": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1560,6 +1571,7 @@ }, "0.8.0": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1569,6 +1581,7 @@ }, "0.8.1": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1578,12 +1591,14 @@ }, "0.8.10": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation" ], "released": "2021-11-09" }, "0.8.11": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "AbiEncodeCallLiteralAsFixedBytesBug" ], @@ -1591,6 +1606,7 @@ }, "0.8.12": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "AbiEncodeCallLiteralAsFixedBytesBug" ], @@ -1598,12 +1614,14 @@ }, "0.8.13": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation" ], "released": "2022-03-16" }, "0.8.2": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory", @@ -1613,6 +1631,7 @@ }, "0.8.3": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables", "ABIDecodeTwoDimensionalArrayMemory" @@ -1621,6 +1640,7 @@ }, "0.8.4": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables" ], @@ -1628,6 +1648,7 @@ }, "0.8.5": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables" ], @@ -1635,6 +1656,7 @@ }, "0.8.6": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables" ], @@ -1642,6 +1664,7 @@ }, "0.8.7": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "SignedImmutables" ], @@ -1649,6 +1672,7 @@ }, "0.8.8": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation", "UserDefinedValueTypesBug", "SignedImmutables" @@ -1657,6 +1681,7 @@ }, "0.8.9": { "bugs": [ + "DataLocationChangeInInternalOverride", "NestedCallataArrayAbiReencodingSizeValidation" ], "released": "2021-09-29" From 0bd0bf4c7d8639f56617bd7b295d07870857ae41 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 17 May 2022 13:49:51 +0200 Subject: [PATCH 11/12] Set release date. --- Changelog.md | 2 +- docs/bugs_by_version.json | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index ebaa17f24..a6c113997 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,4 +1,4 @@ -### 0.8.14 (unreleased) +### 0.8.14 (2022-05-17) Important Bugfixes: * ABI Encoder: When ABI-encoding values from calldata that contain nested arrays, correctly validate the nested array length against ``calldatasize()`` in all cases. diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index d6ee8bb8a..86203a72b 100644 --- a/docs/bugs_by_version.json +++ b/docs/bugs_by_version.json @@ -1619,6 +1619,10 @@ ], "released": "2022-03-16" }, + "0.8.14": { + "bugs": [], + "released": "2022-05-17" + }, "0.8.2": { "bugs": [ "DataLocationChangeInInternalOverride", From 79554b9a408be3f43b388181932fb22ae183be70 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 17 May 2022 18:03:53 +0200 Subject: [PATCH 12/12] Set version to 0.8.15. --- CMakeLists.txt | 2 +- Changelog.md | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7d1ebc53a..426520383 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -21,7 +21,7 @@ include(EthPolicy) eth_policy() # project name and version should be set after cmake_policy CMP0048 -set(PROJECT_VERSION "0.8.14") +set(PROJECT_VERSION "0.8.15") # OSX target needed in order to support std::visit set(CMAKE_OSX_DEPLOYMENT_TARGET "10.14") project(solidity VERSION ${PROJECT_VERSION} LANGUAGES C CXX) diff --git a/Changelog.md b/Changelog.md index a6c113997..b80d6fadf 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,14 @@ +### 0.8.15 (unreleased) + +Language Features: + + +Compiler Features: + + +Bugfixes: + + ### 0.8.14 (2022-05-17) Important Bugfixes: