From 110e2a656da142bdec13f13bf06b9961cc074588 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Mon, 26 Jun 2023 19:07:24 +0200 Subject: [PATCH 1/2] Restrict mobile types of function types. Move ternary tests to semanticTests --- Changelog.md | 1 + libsolidity/ast/Types.cpp | 8 ++++++-- .../functionTypes/selector_ternary.sol | 10 ++++++++++ ...ry_function_pointer_from_function_call.sol | 19 +++++++++++++++++++ .../ternary_contract_internal_function.sol | 10 ++++++++++ ...ary_contract_library_internal_function.sol | 13 +++++++++++++ .../ternary_contract_public_function.sol | 10 ++++++++++ .../errors/error_ternary_operator.sol | 11 +++++++++++ ...error_ternary_operator_different_types.sol | 18 ------------------ .../events/event_ternary_operator.sol | 10 ++++++++++ ...event_ternary_operator_different_types.sol | 17 ----------------- .../functionTypes/from_ternary_expression.sol | 8 ++++++++ ...tor_ternary_contract_external_function.sol | 10 ++++++++++ ...ary_different_contract_public_function.sol | 13 +++++++++++++ ..._different_contracts_external_function.sol | 16 ++++++++++++++++ ..._different_interface_external_function.sol | 16 ++++++++++++++++ 16 files changed, 153 insertions(+), 37 deletions(-) create mode 100644 test/libsolidity/semanticTests/functionTypes/selector_ternary.sol create mode 100644 test/libsolidity/semanticTests/functionTypes/selector_ternary_function_pointer_from_function_call.sol create mode 100644 test/libsolidity/semanticTests/functionTypes/ternary_contract_internal_function.sol create mode 100644 test/libsolidity/semanticTests/functionTypes/ternary_contract_library_internal_function.sol create mode 100644 test/libsolidity/semanticTests/functionTypes/ternary_contract_public_function.sol create mode 100644 test/libsolidity/syntaxTests/errors/error_ternary_operator.sol delete mode 100644 test/libsolidity/syntaxTests/errors/error_ternary_operator_different_types.sol create mode 100644 test/libsolidity/syntaxTests/events/event_ternary_operator.sol delete mode 100644 test/libsolidity/syntaxTests/events/event_ternary_operator_different_types.sol create mode 100644 test/libsolidity/syntaxTests/functionTypes/from_ternary_expression.sol create mode 100644 test/libsolidity/syntaxTests/functionTypes/selector_ternary_contract_external_function.sol create mode 100644 test/libsolidity/syntaxTests/functionTypes/selector_ternary_different_contract_public_function.sol create mode 100644 test/libsolidity/syntaxTests/functionTypes/selector_ternary_different_contracts_external_function.sol create mode 100644 test/libsolidity/syntaxTests/functionTypes/selector_ternary_different_interface_external_function.sol diff --git a/Changelog.md b/Changelog.md index 7b19d37c0..06a761e6e 100644 --- a/Changelog.md +++ b/Changelog.md @@ -30,6 +30,7 @@ Bugfixes: * SMTChecker: Fix false negative when a verification target can be violated only by trusted external call from another public function. * SMTChecker: Fix internal error caused by using external identifier to encode member access to functions that take an internal function as a parameter. * Standard JSON Interface: Fix an incomplete AST being returned when analysis is interrupted by certain kinds of fatal errors. + * Type Checker: Disallow using certain unassignable function types in complex expressions. * Type Checker: Function declaration types referring to different declarations are no longer convertible to each other. * Yul Optimizer: Ensure that the assignment of memory slots for variables moved to memory does not depend on AST IDs that may depend on whether additional files are included during compilation. * Yul Optimizer: Fix ``FullInliner`` step not ignoring code that is not in expression-split form. diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 915440caa..47e1fea04 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -3472,7 +3472,11 @@ Type const* FunctionType::mobileType() const if (valueSet() || gasSet() || saltSet() || hasBoundFirstArgument()) return nullptr; - // return function without parameter names + // Special function types do not get a mobile type, such that they cannot be used in complex expressions. + if (m_kind != FunctionType::Kind::Internal && m_kind != FunctionType::Kind::External && m_kind != FunctionType::Kind::DelegateCall) + return nullptr; + + // return function without parameter names and without declaration return TypeProvider::function( m_parameterTypes, m_returnParameterTypes, @@ -3480,7 +3484,7 @@ Type const* FunctionType::mobileType() const strings(m_returnParameterNames.size()), m_kind, m_stateMutability, - m_declaration, + nullptr, Options::fromFunctionType(*this) ); } diff --git a/test/libsolidity/semanticTests/functionTypes/selector_ternary.sol b/test/libsolidity/semanticTests/functionTypes/selector_ternary.sol new file mode 100644 index 000000000..567b7c14b --- /dev/null +++ b/test/libsolidity/semanticTests/functionTypes/selector_ternary.sol @@ -0,0 +1,10 @@ +contract C { + function f() public {} + function g() public {} + function h(bool c) public returns (bytes4) { + return (c ? this.f : this.g).selector; + } +} +// ---- +// h(bool): true -> 0x26121ff000000000000000000000000000000000000000000000000000000000 +// h(bool): false -> 0xe2179b8e00000000000000000000000000000000000000000000000000000000 diff --git a/test/libsolidity/semanticTests/functionTypes/selector_ternary_function_pointer_from_function_call.sol b/test/libsolidity/semanticTests/functionTypes/selector_ternary_function_pointer_from_function_call.sol new file mode 100644 index 000000000..83415f728 --- /dev/null +++ b/test/libsolidity/semanticTests/functionTypes/selector_ternary_function_pointer_from_function_call.sol @@ -0,0 +1,19 @@ +contract A { + function f() public {} + function g() public {} +} + +contract C { + A a = new A(); + + function getContract() public view returns (A) { + return a; + } + + function test(bool b) public view returns (bytes4) { + return (b ? getContract().f : getContract().g).selector; + } +} +// ---- +// test(bool): true -> 0x26121ff000000000000000000000000000000000000000000000000000000000 +// test(bool): false -> 0xe2179b8e00000000000000000000000000000000000000000000000000000000 diff --git a/test/libsolidity/semanticTests/functionTypes/ternary_contract_internal_function.sol b/test/libsolidity/semanticTests/functionTypes/ternary_contract_internal_function.sol new file mode 100644 index 000000000..f7dc89048 --- /dev/null +++ b/test/libsolidity/semanticTests/functionTypes/ternary_contract_internal_function.sol @@ -0,0 +1,10 @@ +contract C { + function f() internal pure returns(uint256) { return 1;} + function g() internal pure returns(uint256) { return 2; } + function test(bool b) public returns(uint256) { + return (b ? C.f : C.g)(); + } +} +// ---- +// test(bool): true -> 1 +// test(bool): false -> 2 diff --git a/test/libsolidity/semanticTests/functionTypes/ternary_contract_library_internal_function.sol b/test/libsolidity/semanticTests/functionTypes/ternary_contract_library_internal_function.sol new file mode 100644 index 000000000..a29a678d5 --- /dev/null +++ b/test/libsolidity/semanticTests/functionTypes/ternary_contract_library_internal_function.sol @@ -0,0 +1,13 @@ +library L { + function f() internal pure returns(uint256){ return 1; } +} + +contract C { + function g() internal pure returns(uint256) { return 2; } + function test(bool b) public returns(uint256) { + return (b ? L.f : C.g)(); + } +} +// ---- +// test(bool): true -> 1 +// test(bool): false -> 2 diff --git a/test/libsolidity/semanticTests/functionTypes/ternary_contract_public_function.sol b/test/libsolidity/semanticTests/functionTypes/ternary_contract_public_function.sol new file mode 100644 index 000000000..6d52017a5 --- /dev/null +++ b/test/libsolidity/semanticTests/functionTypes/ternary_contract_public_function.sol @@ -0,0 +1,10 @@ +contract C { + function f() public pure returns(uint256) { return 1; } + function g() public pure returns(uint256) { return 2; } + function test(bool b) public returns(uint256) { + return (b ? C.f : C.g)(); + } +} +// ---- +// test(bool): true -> 1 +// test(bool): false -> 2 diff --git a/test/libsolidity/syntaxTests/errors/error_ternary_operator.sol b/test/libsolidity/syntaxTests/errors/error_ternary_operator.sol new file mode 100644 index 000000000..c4600a858 --- /dev/null +++ b/test/libsolidity/syntaxTests/errors/error_ternary_operator.sol @@ -0,0 +1,11 @@ +error MyCustomError(uint, bool); + +contract C { + function f() pure public { + true ? MyCustomError : MyCustomError; + } +} + +// ---- +// TypeError 9717: (93-106): Invalid mobile type in true expression. +// TypeError 3703: (109-122): Invalid mobile type in false expression. diff --git a/test/libsolidity/syntaxTests/errors/error_ternary_operator_different_types.sol b/test/libsolidity/syntaxTests/errors/error_ternary_operator_different_types.sol deleted file mode 100644 index 680ab8058..000000000 --- a/test/libsolidity/syntaxTests/errors/error_ternary_operator_different_types.sol +++ /dev/null @@ -1,18 +0,0 @@ -error MyCustomError(uint, bool); -error MyCustomError2(uint, bool); -error MyCustomError3(uint, bool, bool); - -contract C { - function f() pure public { - true ? MyCustomError : MyCustomError; - true ? MyCustomError : MyCustomError2; - true ? MyCustomError : MyCustomError3; - true ? MyCustomError : true; - true ? true : MyCustomError; - } -} - -// ---- -// TypeError 1080: (253-290): True expression's type error MyCustomError(uint256,bool) does not match false expression's type error MyCustomError3(uint256,bool,bool). -// TypeError 1080: (300-327): True expression's type error MyCustomError(uint256,bool) does not match false expression's type bool. -// TypeError 1080: (337-364): True expression's type bool does not match false expression's type error MyCustomError(uint256,bool). diff --git a/test/libsolidity/syntaxTests/events/event_ternary_operator.sol b/test/libsolidity/syntaxTests/events/event_ternary_operator.sol new file mode 100644 index 000000000..0ba773241 --- /dev/null +++ b/test/libsolidity/syntaxTests/events/event_ternary_operator.sol @@ -0,0 +1,10 @@ +contract C { + event MyCustomEvent(uint); + function f() pure public { + true ? MyCustomEvent : MyCustomEvent; + } +} + +// ---- +// TypeError 9717: (90-103): Invalid mobile type in true expression. +// TypeError 3703: (106-119): Invalid mobile type in false expression. diff --git a/test/libsolidity/syntaxTests/events/event_ternary_operator_different_types.sol b/test/libsolidity/syntaxTests/events/event_ternary_operator_different_types.sol deleted file mode 100644 index ca2f1e1d6..000000000 --- a/test/libsolidity/syntaxTests/events/event_ternary_operator_different_types.sol +++ /dev/null @@ -1,17 +0,0 @@ -contract C { - event MyCustomEvent(uint); - event MyCustomEvent2(uint); - event MyCustomEvent3(uint, bool); - function f() pure public { - true ? MyCustomEvent : MyCustomEvent; - true ? MyCustomEvent : MyCustomEvent2; - true ? MyCustomEvent : MyCustomEvent3; - true ? MyCustomEvent : true; - true ? true : MyCustomEvent; - } -} - -// ---- -// TypeError 1080: (246-283): True expression's type event MyCustomEvent(uint256) does not match false expression's type event MyCustomEvent3(uint256,bool). -// TypeError 1080: (293-320): True expression's type event MyCustomEvent(uint256) does not match false expression's type bool. -// TypeError 1080: (330-357): True expression's type bool does not match false expression's type event MyCustomEvent(uint256). diff --git a/test/libsolidity/syntaxTests/functionTypes/from_ternary_expression.sol b/test/libsolidity/syntaxTests/functionTypes/from_ternary_expression.sol new file mode 100644 index 000000000..62ad8aa93 --- /dev/null +++ b/test/libsolidity/syntaxTests/functionTypes/from_ternary_expression.sol @@ -0,0 +1,8 @@ +contract C { + function f() public pure returns (uint x) { + x = (true ? addmod : addmod)(3, 4, 5); + } +} +// ---- +// TypeError 9717: (81-87): Invalid mobile type in true expression. +// TypeError 3703: (90-96): Invalid mobile type in false expression. diff --git a/test/libsolidity/syntaxTests/functionTypes/selector_ternary_contract_external_function.sol b/test/libsolidity/syntaxTests/functionTypes/selector_ternary_contract_external_function.sol new file mode 100644 index 000000000..847caad06 --- /dev/null +++ b/test/libsolidity/syntaxTests/functionTypes/selector_ternary_contract_external_function.sol @@ -0,0 +1,10 @@ +contract C { + function f() external pure { } + function g() external pure { } + function test(bool b) public returns(bytes4) { + (b ? C.f : C.g).selector; + } +} +// ---- +// TypeError 9717: (147-150): Invalid mobile type in true expression. +// TypeError 3703: (153-156): Invalid mobile type in false expression. diff --git a/test/libsolidity/syntaxTests/functionTypes/selector_ternary_different_contract_public_function.sol b/test/libsolidity/syntaxTests/functionTypes/selector_ternary_different_contract_public_function.sol new file mode 100644 index 000000000..5f5edfa9f --- /dev/null +++ b/test/libsolidity/syntaxTests/functionTypes/selector_ternary_different_contract_public_function.sol @@ -0,0 +1,13 @@ +contract C { + function f() public pure { } + function g() public pure { } +} + +contract A { + function test(bool b) public returns(bytes4) { + (b ? C.f : C.g).selector; + } +} +// ---- +// TypeError 9717: (159-162): Invalid mobile type in true expression. +// TypeError 3703: (165-168): Invalid mobile type in false expression. diff --git a/test/libsolidity/syntaxTests/functionTypes/selector_ternary_different_contracts_external_function.sol b/test/libsolidity/syntaxTests/functionTypes/selector_ternary_different_contracts_external_function.sol new file mode 100644 index 000000000..e015f4315 --- /dev/null +++ b/test/libsolidity/syntaxTests/functionTypes/selector_ternary_different_contracts_external_function.sol @@ -0,0 +1,16 @@ +contract C { + function f() external pure { } +} + +contract D { + function g() external pure { } +} + +contract A { + function test(bool b) public returns(bytes4) { + (b ? C.f : D.g).selector; + } +} +// ---- +// TypeError 9717: (179-182): Invalid mobile type in true expression. +// TypeError 3703: (185-188): Invalid mobile type in false expression. diff --git a/test/libsolidity/syntaxTests/functionTypes/selector_ternary_different_interface_external_function.sol b/test/libsolidity/syntaxTests/functionTypes/selector_ternary_different_interface_external_function.sol new file mode 100644 index 000000000..4109a14ce --- /dev/null +++ b/test/libsolidity/syntaxTests/functionTypes/selector_ternary_different_interface_external_function.sol @@ -0,0 +1,16 @@ +interface I1 { + function f() external pure; +} + +interface I2 { + function g() external pure; +} + +contract C { + function test(bool b) public returns(bytes4) { + (b ? I1.f : I2.g).selector; + } +} +// ---- +// TypeError 9717: (177-181): Invalid mobile type in true expression. +// TypeError 3703: (184-188): Invalid mobile type in false expression. From 82cb5338a90b13a6d9483164bdb7734ed021c86a Mon Sep 17 00:00:00 2001 From: Nikola Matic Date: Tue, 18 Jul 2023 13:55:46 +0200 Subject: [PATCH 2/2] Relax delegatecall type restriction plus test --- libsolidity/ast/Types.cpp | 8 +++----- .../functionTypes/declaration_type_conversion.sol | 3 ++- .../ternary_contract_delegate_function.sol | 12 ++++++++++++ 3 files changed, 17 insertions(+), 6 deletions(-) create mode 100644 test/libsolidity/syntaxTests/functionTypes/ternary_contract_delegate_function.sol diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 47e1fea04..2b955369f 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -3423,12 +3423,10 @@ MemberList::MemberMap FunctionType::nativeMembers(ASTNode const* _scope) const } case Kind::DelegateCall: { - auto const* functionDefinition = dynamic_cast(m_declaration); - solAssert(functionDefinition, ""); - solAssert(functionDefinition->visibility() != Visibility::Private, ""); - if (functionDefinition->visibility() != Visibility::Internal) + if (auto const* functionDefinition = dynamic_cast(m_declaration)) { - auto const* contract = dynamic_cast(m_declaration->scope()); + solAssert(functionDefinition->visibility() > Visibility::Internal, ""); + auto const *contract = dynamic_cast(m_declaration->scope()); solAssert(contract, ""); solAssert(contract->isLibrary(), ""); return {{"selector", TypeProvider::fixedBytes(4)}}; diff --git a/test/libsolidity/syntaxTests/functionTypes/declaration_type_conversion.sol b/test/libsolidity/syntaxTests/functionTypes/declaration_type_conversion.sol index a26460dc1..c31c49aea 100644 --- a/test/libsolidity/syntaxTests/functionTypes/declaration_type_conversion.sol +++ b/test/libsolidity/syntaxTests/functionTypes/declaration_type_conversion.sol @@ -8,4 +8,5 @@ contract C { } } // ---- -// TypeError 1080: (117-130): True expression's type function D.f() does not match false expression's type function D.g(). +// TypeError 9717: (121-124): Invalid mobile type in true expression. +// TypeError 3703: (127-130): Invalid mobile type in false expression. diff --git a/test/libsolidity/syntaxTests/functionTypes/ternary_contract_delegate_function.sol b/test/libsolidity/syntaxTests/functionTypes/ternary_contract_delegate_function.sol new file mode 100644 index 000000000..810f5eb21 --- /dev/null +++ b/test/libsolidity/syntaxTests/functionTypes/ternary_contract_delegate_function.sol @@ -0,0 +1,12 @@ +library L { + function f() external {} +} + +contract C { + function test() public { + (true ? L.f : L.f).selector; + } +} + +// ---- +// TypeError 9582: (94-121): Member "selector" not found or not visible after argument-dependent lookup in function ().