mirror of
https://github.com/ethereum/solidity
synced 2023-10-03 13:03:40 +00:00
Disallow internal function types as parameters for public/external library function
This commit is contained in:
parent
0fbea8a1a0
commit
8e899a0d32
@ -13,6 +13,7 @@ Compiler Features:
|
||||
|
||||
Bugfixes:
|
||||
* Code Generator: Defensively pad memory for ``type(Contract).name`` to multiples of 32.
|
||||
* Type System: Detect and disallow internal function pointers as parameters for public/external library functions, even when they are nested/wrapped in structs, arrays or other types.
|
||||
* Yul Optimizer: Properly determine whether a variable can be eliminated during stack compression pass.
|
||||
|
||||
|
||||
|
@ -355,8 +355,16 @@ bool TypeChecker::visit(FunctionDefinition const& _function)
|
||||
{
|
||||
if (!type(var)->canLiveOutsideStorage() && _function.isPublic())
|
||||
m_errorReporter.typeError(var.location(), "Type is required to live outside storage.");
|
||||
if (_function.isPublic() && !(type(var)->interfaceType(isLibraryFunction).get()))
|
||||
m_errorReporter.fatalTypeError(var.location(), "Internal or recursive type is not allowed for public or external functions.");
|
||||
if (_function.isPublic())
|
||||
{
|
||||
auto iType = type(var)->interfaceType(isLibraryFunction);
|
||||
|
||||
if (!iType.get())
|
||||
{
|
||||
solAssert(!iType.message().empty(), "Expected detailed error message!");
|
||||
m_errorReporter.fatalTypeError(var.location(), iType.message());
|
||||
}
|
||||
}
|
||||
}
|
||||
if (
|
||||
_function.isPublic() &&
|
||||
|
@ -1877,23 +1877,22 @@ TypeResult ArrayType::interfaceType(bool _inLibrary) const
|
||||
if (!_inLibrary && m_interfaceType.is_initialized())
|
||||
return *m_interfaceType;
|
||||
|
||||
TypePointer result;
|
||||
TypeResult result{TypePointer{}};
|
||||
TypeResult baseExt = m_baseType->interfaceType(_inLibrary);
|
||||
|
||||
if (_inLibrary && location() == DataLocation::Storage)
|
||||
if (!baseExt.get())
|
||||
{
|
||||
solAssert(!baseExt.message().empty(), "Expected detailed error message!");
|
||||
result = baseExt;
|
||||
}
|
||||
else if (_inLibrary && location() == DataLocation::Storage)
|
||||
result = shared_from_this();
|
||||
else if (m_arrayKind != ArrayKind::Ordinary)
|
||||
result = this->copyForLocation(DataLocation::Memory, true);
|
||||
else if (isDynamicallySized())
|
||||
result = TypePointer{make_shared<ArrayType>(DataLocation::Memory, baseExt)};
|
||||
else
|
||||
{
|
||||
TypePointer baseExt = m_baseType->interfaceType(_inLibrary);
|
||||
|
||||
if (!baseExt)
|
||||
result = TypePointer();
|
||||
else if (isDynamicallySized())
|
||||
result = make_shared<ArrayType>(DataLocation::Memory, baseExt);
|
||||
else
|
||||
result = make_shared<ArrayType>(DataLocation::Memory, baseExt, m_length);
|
||||
}
|
||||
result = TypePointer{make_shared<ArrayType>(DataLocation::Memory, baseExt, m_length)};
|
||||
|
||||
if (_inLibrary)
|
||||
m_interfaceType_library = result;
|
||||
@ -2084,7 +2083,7 @@ unsigned StructType::calldataOffsetOfMember(std::string const& _member) const
|
||||
|
||||
bool StructType::isDynamicallyEncoded() const
|
||||
{
|
||||
solAssert(!recursive(), "");
|
||||
solAssert(interfaceType(false).get(), "");
|
||||
for (auto t: memoryMemberTypes())
|
||||
{
|
||||
solAssert(t, "Parameter should have external type.");
|
||||
@ -2143,47 +2142,86 @@ TypeResult StructType::interfaceType(bool _inLibrary) const
|
||||
if (!_inLibrary && m_interfaceType.is_initialized())
|
||||
return *m_interfaceType;
|
||||
|
||||
TypePointer result = TypePointer{};
|
||||
bool recursion = false;
|
||||
TypeResult result{TypePointer{}};
|
||||
|
||||
if (_inLibrary && location() == DataLocation::Storage)
|
||||
result = shared_from_this();
|
||||
else if (recursive())
|
||||
result = TypePointer{};
|
||||
else
|
||||
auto visitor = [&](
|
||||
StructDefinition const& _struct,
|
||||
CycleDetector<StructDefinition>& _cycleDetector,
|
||||
size_t /*_depth*/
|
||||
)
|
||||
{
|
||||
// Check that all members have interface types.
|
||||
// We pass "false" to canBeUsedExternally (_inLibrary), because this struct will be
|
||||
// passed by value and thus the encoding does not differ, but it will disallow
|
||||
// mappings.
|
||||
// Also return false 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,
|
||||
// which is reported as an error.
|
||||
bool allOkay = true;
|
||||
for (auto const& var: m_struct.members())
|
||||
// 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<VariableDeclaration> const& variable: _struct.members())
|
||||
{
|
||||
// If the struct member does not have a type return false.
|
||||
// A TypeError is expected in this case.
|
||||
if (!var->annotation().type)
|
||||
if (!variable->annotation().type)
|
||||
{
|
||||
allOkay = false;
|
||||
break;
|
||||
result = TypeResult::err("Invalid type!");
|
||||
return;
|
||||
}
|
||||
if (!var->annotation().type->interfaceType(false).get())
|
||||
|
||||
Type const* memberType = variable->annotation().type.get();
|
||||
|
||||
while (dynamic_cast<ArrayType const*>(memberType))
|
||||
memberType = dynamic_cast<ArrayType const*>(memberType)->baseType().get();
|
||||
|
||||
if (StructType const* innerStruct = dynamic_cast<StructType const*>(memberType))
|
||||
if (
|
||||
innerStruct->m_recursive == true ||
|
||||
_cycleDetector.run(innerStruct->structDefinition())
|
||||
)
|
||||
{
|
||||
recursion = true;
|
||||
if (_inLibrary && location() == DataLocation::Storage)
|
||||
continue;
|
||||
else
|
||||
{
|
||||
result = TypeResult::err("Recursive structs can only be passed as storage pointers to libraries, not as memory objects to contract functions.");
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
auto iType = memberType->interfaceType(_inLibrary);
|
||||
if (!iType.get())
|
||||
{
|
||||
allOkay = false;
|
||||
break;
|
||||
solAssert(!iType.message().empty(), "Expected detailed error message!");
|
||||
result = iType;
|
||||
return;
|
||||
}
|
||||
}
|
||||
if (allOkay)
|
||||
result = copyForLocation(DataLocation::Memory, true);
|
||||
}
|
||||
};
|
||||
|
||||
recursion = recursion || (CycleDetector<StructDefinition>(visitor).run(structDefinition()) != nullptr);
|
||||
|
||||
std::string const recursiveErrMsg = "Recursive type not allowed for public or external contract functions.";
|
||||
|
||||
if (_inLibrary)
|
||||
m_interfaceType_library = result;
|
||||
else
|
||||
m_interfaceType = result;
|
||||
{
|
||||
if (!result.message().empty())
|
||||
m_interfaceType_library = result;
|
||||
else if (location() == DataLocation::Storage)
|
||||
m_interfaceType_library = shared_from_this();
|
||||
else
|
||||
m_interfaceType_library = copyForLocation(DataLocation::Memory, true);
|
||||
|
||||
return result;
|
||||
if (recursion)
|
||||
m_interfaceType = TypeResult::err(recursiveErrMsg);
|
||||
|
||||
return *m_interfaceType_library;
|
||||
}
|
||||
|
||||
if (recursion)
|
||||
m_interfaceType = TypeResult::err(recursiveErrMsg);
|
||||
else if (!result.message().empty())
|
||||
m_interfaceType = result;
|
||||
else
|
||||
m_interfaceType = copyForLocation(DataLocation::Memory, true);
|
||||
|
||||
return *m_interfaceType;
|
||||
}
|
||||
|
||||
TypePointer StructType::copyForLocation(DataLocation _location, bool _isPointer) const
|
||||
@ -2273,27 +2311,6 @@ set<string> StructType::membersMissingInMemory() const
|
||||
return missing;
|
||||
}
|
||||
|
||||
bool StructType::recursive() const
|
||||
{
|
||||
if (!m_recursive.is_initialized())
|
||||
{
|
||||
auto visitor = [&](StructDefinition const& _struct, CycleDetector<StructDefinition>& _cycleDetector, size_t /*_depth*/)
|
||||
{
|
||||
for (ASTPointer<VariableDeclaration> const& variable: _struct.members())
|
||||
{
|
||||
Type const* memberType = variable->annotation().type.get();
|
||||
while (dynamic_cast<ArrayType const*>(memberType))
|
||||
memberType = dynamic_cast<ArrayType const*>(memberType)->baseType().get();
|
||||
if (StructType const* innerStruct = dynamic_cast<StructType const*>(memberType))
|
||||
if (_cycleDetector.run(innerStruct->structDefinition()))
|
||||
return;
|
||||
}
|
||||
};
|
||||
m_recursive = (CycleDetector<StructDefinition>(visitor).run(structDefinition()) != nullptr);
|
||||
}
|
||||
return *m_recursive;
|
||||
}
|
||||
|
||||
TypeResult EnumType::unaryOperatorResult(Token _operator) const
|
||||
{
|
||||
return _operator == Token::Delete ? make_shared<TupleType>() : TypePointer();
|
||||
@ -2983,7 +3000,7 @@ TypeResult FunctionType::interfaceType(bool /*_inLibrary*/) const
|
||||
if (m_kind == Kind::External)
|
||||
return shared_from_this();
|
||||
else
|
||||
return TypePointer();
|
||||
return TypeResult::err("Internal type is not allowed for public or external functions.");
|
||||
}
|
||||
|
||||
bool FunctionType::canTakeArguments(
|
||||
@ -3277,6 +3294,26 @@ string MappingType::canonicalName() const
|
||||
return "mapping(" + keyType()->canonicalName() + " => " + valueType()->canonicalName() + ")";
|
||||
}
|
||||
|
||||
TypeResult MappingType::interfaceType(bool _inLibrary) const
|
||||
{
|
||||
solAssert(keyType()->interfaceType(_inLibrary).get(), "Must be an elementary type!");
|
||||
|
||||
if (_inLibrary)
|
||||
{
|
||||
auto iType = valueType()->interfaceType(_inLibrary);
|
||||
|
||||
if (!iType.get())
|
||||
{
|
||||
solAssert(!iType.message().empty(), "Expected detailed error message!");
|
||||
return iType;
|
||||
}
|
||||
}
|
||||
else
|
||||
return TypeResult::err("Only libraries are allowed to use the mapping type in public or external functions.");
|
||||
|
||||
return shared_from_this();
|
||||
}
|
||||
|
||||
string TypeType::richIdentifier() const
|
||||
{
|
||||
return "t_type" + identifierList(actualType());
|
||||
|
@ -863,15 +863,9 @@ public:
|
||||
TypePointers memoryMemberTypes() const;
|
||||
/// @returns the set of all members that are removed in the memory version (typically mappings).
|
||||
std::set<std::string> membersMissingInMemory() const;
|
||||
|
||||
/// @returns true if the same struct is used recursively in one of its members. Only
|
||||
/// analyses the "memory" representation, i.e. mappings are ignored in all structs.
|
||||
bool recursive() const;
|
||||
|
||||
private:
|
||||
StructDefinition const& m_struct;
|
||||
/// Cache for the recursive() function.
|
||||
mutable boost::optional<bool> m_recursive;
|
||||
// Caches for interfaceType(bool)
|
||||
mutable boost::optional<TypeResult> m_interfaceType;
|
||||
mutable boost::optional<TypeResult> m_interfaceType_library;
|
||||
};
|
||||
@ -1223,10 +1217,7 @@ public:
|
||||
{
|
||||
return std::make_shared<IntegerType>(256);
|
||||
}
|
||||
TypeResult interfaceType(bool _inLibrary) const override
|
||||
{
|
||||
return _inLibrary ? shared_from_this() : TypePointer();
|
||||
}
|
||||
TypeResult interfaceType(bool _inLibrary) const override;
|
||||
bool dataStoredIn(DataLocation _location) const override { return _location == DataLocation::Storage; }
|
||||
/// Cannot be stored in memory, but just in case.
|
||||
bool hasSimpleZeroValueInMemory() const override { solAssert(false, ""); }
|
||||
|
@ -0,0 +1,6 @@
|
||||
library L {
|
||||
// Used to cause internal error
|
||||
function f(function(uint) internal returns (uint)[] memory x) public { }
|
||||
}
|
||||
// ----
|
||||
// TypeError: (63-112): Internal type is not allowed for public or external functions.
|
@ -0,0 +1,6 @@
|
||||
library L {
|
||||
// Used to cause internal error
|
||||
function g(function(uint) internal returns (uint)[] storage x) public { }
|
||||
}
|
||||
// ----
|
||||
// TypeError: (63-113): Internal type is not allowed for public or external functions.
|
@ -5,4 +5,4 @@ contract C {
|
||||
}
|
||||
}
|
||||
// ----
|
||||
// TypeError: (124-164): Internal or recursive type is not allowed for public or external functions.
|
||||
// TypeError: (124-164): Internal type is not allowed for public or external functions.
|
||||
|
@ -3,4 +3,4 @@ library L {
|
||||
}
|
||||
}
|
||||
// ----
|
||||
// TypeError: (27-67): Internal or recursive type is not allowed for public or external functions.
|
||||
// TypeError: (27-67): Internal type is not allowed for public or external functions.
|
||||
|
@ -4,4 +4,4 @@ contract C {
|
||||
}
|
||||
}
|
||||
// ----
|
||||
// TypeError: (129-169): Internal or recursive type is not allowed for public or external functions.
|
||||
// TypeError: (129-169): Internal type is not allowed for public or external functions.
|
||||
|
@ -0,0 +1,9 @@
|
||||
library L {
|
||||
struct S
|
||||
{
|
||||
function(uint) internal returns (uint)[] x;
|
||||
}
|
||||
function f(S storage s) public { }
|
||||
}
|
||||
// ----
|
||||
// TypeError: (104-115): Internal type is not allowed for public or external functions.
|
@ -6,4 +6,4 @@ contract C {
|
||||
}
|
||||
// ----
|
||||
// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments.
|
||||
// TypeError: (103-111): Internal or recursive type is not allowed for public or external functions.
|
||||
// TypeError: (103-111): Internal type is not allowed for public or external functions.
|
||||
|
@ -6,4 +6,4 @@ contract C {
|
||||
}
|
||||
// ----
|
||||
// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments.
|
||||
// TypeError: (105-113): Internal or recursive type is not allowed for public or external functions.
|
||||
// TypeError: (105-113): Only libraries are allowed to use the mapping type in public or external functions.
|
||||
|
@ -7,4 +7,4 @@ contract C {
|
||||
}
|
||||
// ----
|
||||
// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments.
|
||||
// TypeError: (132-140): Internal or recursive type is not allowed for public or external functions.
|
||||
// TypeError: (132-140): Only libraries are allowed to use the mapping type in public or external functions.
|
||||
|
@ -0,0 +1,10 @@
|
||||
contract Test {
|
||||
struct MyStructName {
|
||||
address addr;
|
||||
MyStructName[] x;
|
||||
}
|
||||
|
||||
function f(MyStructName memory s) public {}
|
||||
}
|
||||
// ----
|
||||
// TypeError: (112-133): Recursive type not allowed for public or external contract functions.
|
@ -0,0 +1,9 @@
|
||||
library Test {
|
||||
struct MyStructName {
|
||||
address addr;
|
||||
MyStructName[] x;
|
||||
}
|
||||
|
||||
function f(MyStructName storage s) public {}
|
||||
}
|
||||
// ----
|
@ -0,0 +1,14 @@
|
||||
pragma experimental ABIEncoderV2;
|
||||
|
||||
library Test {
|
||||
struct MyStructName {
|
||||
address addr;
|
||||
MyStructName[] x;
|
||||
}
|
||||
|
||||
function f(MyStructName memory _x) public {
|
||||
}
|
||||
}
|
||||
// ----
|
||||
// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments.
|
||||
// TypeError: (146-168): Recursive structs can only be passed as storage pointers to libraries, not as memory objects to contract functions.
|
@ -8,4 +8,4 @@ contract Data {
|
||||
}
|
||||
// ----
|
||||
// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments.
|
||||
// TypeError: (63-78): Internal or recursive type is not allowed for public or external functions.
|
||||
// TypeError: (63-78): Recursive type not allowed for public or external contract functions.
|
||||
|
@ -0,0 +1,11 @@
|
||||
library Test {
|
||||
struct MyStructName {
|
||||
address addr;
|
||||
MyStructName[] x;
|
||||
function() internal y;
|
||||
}
|
||||
|
||||
function f(MyStructName storage s) public {}
|
||||
}
|
||||
// ----
|
||||
// TypeError: (142-164): Internal type is not allowed for public or external functions.
|
@ -4,4 +4,4 @@ contract C {
|
||||
}
|
||||
}
|
||||
// ----
|
||||
// TypeError: (91-99): Internal or recursive type is not allowed for public or external functions.
|
||||
// TypeError: (91-99): Recursive type not allowed for public or external contract functions.
|
||||
|
@ -4,4 +4,4 @@ contract C {
|
||||
}
|
||||
}
|
||||
// ----
|
||||
// TypeError: (94-102): Internal or recursive type is not allowed for public or external functions.
|
||||
// TypeError: (94-102): Recursive type not allowed for public or external contract functions.
|
||||
|
@ -5,4 +5,4 @@ contract C {
|
||||
}
|
||||
}
|
||||
// ----
|
||||
// TypeError: (119-129): Internal or recursive type is not allowed for public or external functions.
|
||||
// TypeError: (119-129): Recursive type not allowed for public or external contract functions.
|
||||
|
@ -3,4 +3,4 @@ contract c {
|
||||
}
|
||||
// ----
|
||||
// TypeError: (29-61): Type is required to live outside storage.
|
||||
// TypeError: (29-61): Internal or recursive type is not allowed for public or external functions.
|
||||
// TypeError: (29-61): Only libraries are allowed to use the mapping type in public or external functions.
|
||||
|
Loading…
Reference in New Issue
Block a user