diff --git a/Changelog.md b/Changelog.md index 7f0f505f0..a344d516d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,8 @@ ### 0.6.10 (unreleased) +Important Bugfixes: + * Fixed a bug related to internal library functions with ``calldata`` parameters called via ``using for``. + Language Features: Compiler Features: diff --git a/docs/bugs.json b/docs/bugs.json index 90f211c1f..3a29ad352 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -1,4 +1,12 @@ [ + { + "name": "UsingForCalldata", + "summary": "Function calls to internal library functions with calldata parameters called via ``using for`` can result in invalid data being read.", + "description": "Function calls to internal library functions using the ``using for`` mechanism copied all calldata parameters to memory first and passed them on like that, regardless of whether it was an internal or an external call. Due to that, the called function would receive a memory pointer that is interpreted as a calldata pointer.", + "introduced": "0.6.9", + "fixed": "0.6.10", + "severity": "very low" + }, { "name": "MissingEscapingInFormatting", "summary": "String literals containing double backslash characters passed directly to external or encoding function calls can lead to a different string being used when ABIEncoderV2 is enabled.", diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index 2d81e8783..37d4419e7 100644 --- a/docs/bugs_by_version.json +++ b/docs/bugs_by_version.json @@ -1165,7 +1165,9 @@ "released": "2020-05-14" }, "0.6.9": { - "bugs": [], + "bugs": [ + "UsingForCalldata" + ], "released": "2020-06-04" } } \ No newline at end of file diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 8114be800..828ff7815 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -371,7 +371,8 @@ MemberList::MemberMap Type::boundFunctions(Type const& _type, ContractDefinition seenFunctions.insert(function); if (function->parameters().empty()) continue; - FunctionTypePointer fun = FunctionType(*function, FunctionType::Kind::External).asExternallyCallableFunction(true, true); + FunctionTypePointer fun = + dynamic_cast(*function->typeViaContractName()).asBoundFunction(); if (_type.isImplicitlyConvertibleTo(*fun->selfType())) members.emplace_back(function->name(), fun, function); } @@ -3438,11 +3439,32 @@ TypePointer FunctionType::copyAndSetCallOptions(bool _setGas, bool _setValue, bo ); } -FunctionTypePointer FunctionType::asExternallyCallableFunction(bool _inLibrary, bool _bound) const +FunctionTypePointer FunctionType::asBoundFunction() const { - if (_bound) - solAssert(!m_parameterTypes.empty(), ""); + solAssert(!m_parameterTypes.empty(), ""); + FunctionDefinition const* fun = dynamic_cast(m_declaration); + solAssert(fun && fun->libraryFunction(), ""); + solAssert(!m_gasSet, ""); + solAssert(!m_valueSet, ""); + solAssert(!m_saltSet, ""); + return TypeProvider::function( + m_parameterTypes, + m_returnParameterTypes, + m_parameterNames, + m_returnParameterNames, + m_kind, + m_arbitraryParameters, + m_stateMutability, + m_declaration, + m_gasSet, + m_valueSet, + m_saltSet, + true + ); +} +FunctionTypePointer FunctionType::asExternallyCallableFunction(bool _inLibrary) const +{ TypePointers parameterTypes; for (auto const& t: m_parameterTypes) if (TypeProvider::isReferenceWithLocation(t, DataLocation::CallData)) @@ -3465,10 +3487,8 @@ FunctionTypePointer FunctionType::asExternallyCallableFunction(bool _inLibrary, if (_inLibrary) { solAssert(!!m_declaration, "Declaration has to be available."); - if (!m_declaration->isPublic()) - kind = Kind::Internal; // will be inlined - else - kind = Kind::DelegateCall; + solAssert(m_declaration->isPublic(), ""); + kind = Kind::DelegateCall; } return TypeProvider::function( @@ -3483,7 +3503,7 @@ FunctionTypePointer FunctionType::asExternallyCallableFunction(bool _inLibrary, m_gasSet, m_valueSet, m_saltSet, - _bound + m_bound ); } diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index 609f2e9ed..1b37dc485 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -1302,13 +1302,16 @@ public: /// of the parameters to false. TypePointer copyAndSetCallOptions(bool _setGas, bool _setValue, bool _setSalt) const; + /// @returns a copy of this function type with the `bound` flag set to true. + /// Should only be called on library functions. + FunctionTypePointer asBoundFunction() const; + /// @returns a copy of this function type where the location of reference types is changed /// from CallData to Memory. This is the type that would be used when the function is /// called externally, as opposed to the parameter types that are available inside the function body. /// Also supports variants to be used for library or bound calls. /// @param _inLibrary if true, uses DelegateCall as location. - /// @param _bound if true, the function type is set to be bound. - FunctionTypePointer asExternallyCallableFunction(bool _inLibrary, bool _bound = false) const; + FunctionTypePointer asExternallyCallableFunction(bool _inLibrary) const; protected: std::vector> makeStackItems() const override; diff --git a/test/libsolidity/semanticTests/libraries/bound_returning_calldata.sol b/test/libsolidity/semanticTests/libraries/bound_returning_calldata.sol new file mode 100644 index 000000000..7451e9dc4 --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/bound_returning_calldata.sol @@ -0,0 +1,17 @@ +library D { + function f(bytes calldata _x) internal pure returns (bytes calldata) { + return _x; + } + function g(bytes calldata _x) internal pure returns (bytes memory) { + return _x; + } +} + +contract C { + using D for bytes; + function f(bytes calldata _x) public pure returns (byte, byte) { + return (_x.f()[0], _x.g()[0]); + } +} +// ---- +// f(bytes): 0x20, 4, "abcd" -> 0x6100000000000000000000000000000000000000000000000000000000000000, 0x6100000000000000000000000000000000000000000000000000000000000000 diff --git a/test/libsolidity/semanticTests/libraries/bound_returning_calldata_external.sol b/test/libsolidity/semanticTests/libraries/bound_returning_calldata_external.sol new file mode 100644 index 000000000..3114f8510 --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/bound_returning_calldata_external.sol @@ -0,0 +1,20 @@ +library D { + function f(bytes calldata _x) public pure returns (bytes calldata) { + return _x; + } + function g(bytes calldata _x) public pure returns (bytes memory) { + return _x; + } +} + +contract C { + using D for bytes; + function f(bytes calldata _x) public pure returns (byte, byte) { + return (_x.f()[0], _x.g()[0]); + } +} +// ==== +// EVMVersion: >homestead +// ---- +// library: D +// f(bytes): 0x20, 4, "abcd" -> 0x6100000000000000000000000000000000000000000000000000000000000000, 0x6100000000000000000000000000000000000000000000000000000000000000 diff --git a/test/libsolidity/semanticTests/libraries/bound_to_calldata.sol b/test/libsolidity/semanticTests/libraries/bound_to_calldata.sol new file mode 100644 index 000000000..0e9bb811b --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/bound_to_calldata.sol @@ -0,0 +1,17 @@ +library D { + function f(bytes calldata _x) internal pure returns (byte) { + return _x[0]; + } + function g(bytes memory _x) internal pure returns (byte) { + return _x[0]; + } +} + +contract C { + using D for bytes; + function f(bytes calldata _x) public pure returns (byte, byte) { + return (_x.f(), _x.g()); + } +} +// ---- +// f(bytes): 0x20, 4, "abcd" -> 0x6100000000000000000000000000000000000000000000000000000000000000, 0x6100000000000000000000000000000000000000000000000000000000000000 diff --git a/test/libsolidity/semanticTests/libraries/bound_to_calldata_external.sol b/test/libsolidity/semanticTests/libraries/bound_to_calldata_external.sol new file mode 100644 index 000000000..2f8c7c7f3 --- /dev/null +++ b/test/libsolidity/semanticTests/libraries/bound_to_calldata_external.sol @@ -0,0 +1,20 @@ +library D { + function f(bytes calldata _x) public pure returns (byte) { + return _x[0]; + } + function g(bytes memory _x) public pure returns (byte) { + return _x[0]; + } +} + +contract C { + using D for bytes; + function f(bytes calldata _x) public pure returns (byte, byte) { + return (_x.f(), _x.g()); + } +} +// ==== +// EVMVersion: >homestead +// ---- +// library: D +// f(bytes): 0x20, 4, "abcd" -> 0x6100000000000000000000000000000000000000000000000000000000000000, 0x6100000000000000000000000000000000000000000000000000000000000000 diff --git a/test/libsolidity/syntaxTests/bound/bound_calldata.sol b/test/libsolidity/syntaxTests/bound/bound_calldata.sol new file mode 100644 index 000000000..f6fcf82cb --- /dev/null +++ b/test/libsolidity/syntaxTests/bound/bound_calldata.sol @@ -0,0 +1,9 @@ +library D { function f(bytes calldata) internal pure {} } +contract C { + using D for bytes; + function f(bytes memory _x) public pure { + _x.f(); + } +} +// ---- +// TypeError: (136-140): Member "f" not found or not visible after argument-dependent lookup in bytes memory.