From 78265642262cb50a3136de024ffa0f0f52f58c3c Mon Sep 17 00:00:00 2001 From: Harikrishnan Mulackal Date: Thu, 27 Aug 2020 10:00:08 +0200 Subject: [PATCH] Fix infinite loop for structs in library function parameter --- Changelog.md | 1 + libsolidity/ast/Types.cpp | 82 ++++++++++--------- ...recursive_struct_nested_mapping_memory.sol | 10 +++ ...ecursive_struct_nested_mapping_storage.sol | 6 ++ 4 files changed, 60 insertions(+), 39 deletions(-) create mode 100644 test/libsolidity/syntaxTests/structs/recursion/recursive_struct_nested_mapping_memory.sol create mode 100644 test/libsolidity/syntaxTests/structs/recursion/recursive_struct_nested_mapping_storage.sol diff --git a/Changelog.md b/Changelog.md index cbce12cba..cf3989662 100644 --- a/Changelog.md +++ b/Changelog.md @@ -22,6 +22,7 @@ Bugfixes: * Yul Optimizer: Make function inlining order more resilient to whether or not unrelated source files are present. * Type Checker: Disallow signed literals as exponent in exponentiation operator. * Allow `type(Contract).name` for abstract contracts and interfaces. + * Type Checker: Disallow structs containing nested mapping in memory as parameters for library functions. ### 0.7.0 (2020-07-28) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 63c7e7221..52b206737 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -2463,52 +2463,56 @@ TypeResult StructType::interfaceType(bool _inLibrary) const TypeResult result{TypePointer{}}; + if (recursive() && !(_inLibrary && location() == DataLocation::Storage)) + return TypeResult::err( + "Recursive structs can only be passed as storage pointers to libraries, " + "not as memory objects to contract functions." + ); + util::BreadthFirstSearch breadthFirstSearch{{&m_struct}}; breadthFirstSearch.run( - [&](StructDefinition const* _struct, auto&& _addChild) { - // Check that all members have interface types. - // Return an error if at least one struct member does not have a type. - // This might happen, for example, if the type of the member does not exist. - for (ASTPointer const& variable: _struct->members()) + [&](StructDefinition const* _struct, auto&& _addChild) + { + // Check that all members have interface types. + // Return an error if at least one struct member does not have a type. + // This might happen, for example, if the type of the member does not exist. + for (ASTPointer const& variable: _struct->members()) + { + // If the struct member does not have a type return false. + // A TypeError is expected in this case. + if (!variable->annotation().type) { - // If the struct member does not have a type return false. - // A TypeError is expected in this case. - if (!variable->annotation().type) + result = TypeResult::err("Invalid type!"); + breadthFirstSearch.abort(); + return; + } + + Type const* memberType = variable->annotation().type; + + while ( + memberType->category() == Type::Category::Array || + memberType->category() == Type::Category::Mapping + ) + { + if (auto arrayType = dynamic_cast(memberType)) + memberType = arrayType->finalBaseType(false); + else if (auto mappingType = dynamic_cast(memberType)) + memberType = mappingType->valueType(); + } + + if (StructType const* innerStruct = dynamic_cast(memberType)) + _addChild(&innerStruct->structDefinition()); + else + { + auto iType = memberType->interfaceType(_inLibrary); + if (!iType.get()) { - result = TypeResult::err("Invalid type!"); + solAssert(!iType.message().empty(), "Expected detailed error message!"); + result = iType; breadthFirstSearch.abort(); return; } - - Type const* memberType = variable->annotation().type; - - while (dynamic_cast(memberType)) - memberType = dynamic_cast(memberType)->baseType(); - - if (StructType const* innerStruct = dynamic_cast(memberType)) - { - if (innerStruct->recursive() && !(_inLibrary && location() == DataLocation::Storage)) - { - result = TypeResult::err( - "Recursive structs can only be passed as storage pointers to libraries, not as memory objects to contract functions." - ); - breadthFirstSearch.abort(); - return; - } - else - _addChild(&innerStruct->structDefinition()); - } - else - { - auto iType = memberType->interfaceType(_inLibrary); - if (!iType.get()) - { - solAssert(!iType.message().empty(), "Expected detailed error message!"); - result = iType; - breadthFirstSearch.abort(); - return; - } - } + } } } ); diff --git a/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_nested_mapping_memory.sol b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_nested_mapping_memory.sol new file mode 100644 index 000000000..6009ee94d --- /dev/null +++ b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_nested_mapping_memory.sol @@ -0,0 +1,10 @@ +library a { + struct b { + mapping (uint => b) c ; + } + // Segfaults in https://github.com/ethereum/solidity/issues/9443 + function d(b memory) public {} +} +// ---- +// TypeError 4103: (149-157): Recursive structs can only be passed as storage pointers to libraries, not as memory objects to contract functions. +// TypeError 4061: (149-157): Type struct a.b is only valid in storage because it contains a (nested) mapping. diff --git a/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_nested_mapping_storage.sol b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_nested_mapping_storage.sol new file mode 100644 index 000000000..d40a4407c --- /dev/null +++ b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_nested_mapping_storage.sol @@ -0,0 +1,6 @@ +library a { + struct b { + mapping (uint => b) c ; + } + function d(b storage) public {} +}