Merge pull request #6222 from ethereum/internal-error-maybe

Disallow internal function types as parameters for public/external library function
This commit is contained in:
chriseth 2019-03-21 09:18:23 +01:00 committed by GitHub
commit 51ae865b96
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
24 changed files with 230 additions and 116 deletions

View File

@ -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.

View File

@ -213,7 +213,7 @@ void ReferencesResolver::endVisit(FunctionTypeName const& _typeName)
for (auto const& t: _typeName.parameterTypes() + _typeName.returnParameterTypes())
{
solAssert(t->annotation().type, "Type not set for parameter.");
if (!t->annotation().type->interfaceType(false))
if (!t->annotation().type->interfaceType(false).get())
{
fatalTypeError(t->location(), "Internal type cannot be used for external function type.");
return;

View File

@ -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)))
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() &&
@ -576,7 +584,7 @@ bool TypeChecker::visit(EventDefinition const& _eventDef)
numIndexed++;
if (!type(*var)->canLiveOutsideStorage())
m_errorReporter.typeError(var->location(), "Type is required to live outside storage.");
if (!type(*var)->interfaceType(false))
if (!type(*var)->interfaceType(false).get())
m_errorReporter.typeError(var->location(), "Internal or recursive type is not allowed as event parameter type.");
if (
!_eventDef.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2) &&
@ -599,7 +607,7 @@ void TypeChecker::endVisit(FunctionTypeName const& _funType)
{
FunctionType const& fun = dynamic_cast<FunctionType const&>(*_funType.annotation().type);
if (fun.kind() == FunctionType::Kind::External)
solAssert(fun.interfaceType(false), "External function type uses internal types.");
solAssert(fun.interfaceType(false).get(), "External function type uses internal types.");
}
bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)

View File

@ -1869,7 +1869,7 @@ TypePointer ArrayType::decodingType() const
return shared_from_this();
}
TypePointer ArrayType::interfaceType(bool _inLibrary) const
TypeResult ArrayType::interfaceType(bool _inLibrary) const
{
if (_inLibrary && m_interfaceType_library.is_initialized())
return *m_interfaceType_library;
@ -1877,23 +1877,22 @@ TypePointer ArrayType::interfaceType(bool _inLibrary) const
if (!_inLibrary && m_interfaceType.is_initialized())
return *m_interfaceType;
TypePointer result;
TypeResult result{TypePointer{}};
TypeResult baseInterfaceType = m_baseType->interfaceType(_inLibrary);
if (_inLibrary && location() == DataLocation::Storage)
if (!baseInterfaceType.get())
{
solAssert(!baseInterfaceType.message().empty(), "Expected detailed error message!");
result = baseInterfaceType;
}
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, baseInterfaceType)};
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, baseInterfaceType, 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.");
@ -2135,7 +2134,7 @@ MemberList::MemberMap StructType::nativeMembers(ContractDefinition const*) const
return members;
}
TypePointer StructType::interfaceType(bool _inLibrary) const
TypeResult StructType::interfaceType(bool _inLibrary) const
{
if (_inLibrary && m_interfaceType_library.is_initialized())
return *m_interfaceType_library;
@ -2143,47 +2142,87 @@ TypePointer StructType::interfaceType(bool _inLibrary) const
if (!_inLibrary && m_interfaceType.is_initialized())
return *m_interfaceType;
TypePointer result = TypePointer{};
TypeResult result{TypePointer{}};
if (_inLibrary && location() == DataLocation::Storage)
result = shared_from_this();
else if (recursive())
result = TypePointer{};
else
m_recursive = false;
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))
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())
)
{
m_recursive = 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);
}
};
m_recursive = m_recursive.get() || (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 (m_recursive.get())
m_interfaceType = TypeResult::err(recursiveErrMsg);
return *m_interfaceType_library;
}
if (m_recursive.get())
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
@ -2204,8 +2243,8 @@ string StructType::signatureInExternalFunction(bool _structsByName) const
{
solAssert(_t, "Parameter should have external type.");
auto t = _t->interfaceType(_structsByName);
solAssert(t, "");
return t->signatureInExternalFunction(_structsByName);
solAssert(t.get(), "");
return t.get()->signatureInExternalFunction(_structsByName);
});
return "(" + boost::algorithm::join(memberTypeStrings, ",") + ")";
}
@ -2273,27 +2312,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();
@ -2574,7 +2592,7 @@ FunctionType::FunctionType(FunctionTypeName const& _typeName):
solAssert(t->annotation().type, "Type not set for parameter.");
if (m_kind == Kind::External)
solAssert(
t->annotation().type->interfaceType(false),
t->annotation().type->interfaceType(false).get(),
"Internal type used as parameter for external function."
);
m_parameterTypes.push_back(t->annotation().type);
@ -2584,7 +2602,7 @@ FunctionType::FunctionType(FunctionTypeName const& _typeName):
solAssert(t->annotation().type, "Type not set for return parameter.");
if (m_kind == Kind::External)
solAssert(
t->annotation().type->interfaceType(false),
t->annotation().type->interfaceType(false).get(),
"Internal type used as return parameter for external function."
);
m_returnParameterTypes.push_back(t->annotation().type);
@ -2885,14 +2903,14 @@ FunctionTypePointer FunctionType::interfaceFunctionType() const
for (auto type: m_parameterTypes)
{
if (auto ext = type->interfaceType(isLibraryFunction))
if (auto ext = type->interfaceType(isLibraryFunction).get())
paramTypes.push_back(ext);
else
return FunctionTypePointer();
}
for (auto type: m_returnParameterTypes)
{
if (auto ext = type->interfaceType(isLibraryFunction))
if (auto ext = type->interfaceType(isLibraryFunction).get())
retParamTypes.push_back(ext);
else
return FunctionTypePointer();
@ -2978,12 +2996,12 @@ TypePointer FunctionType::encodingType() const
return TypePointer();
}
TypePointer FunctionType::interfaceType(bool /*_inLibrary*/) const
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 +3295,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());

View File

@ -304,7 +304,7 @@ public:
/// If there is no such type, returns an empty shared pointer.
/// @param _inLibrary if set, returns types as used in a library, e.g. struct and contract types
/// are returned without modification.
virtual TypePointer interfaceType(bool /*_inLibrary*/) const { return TypePointer(); }
virtual TypeResult interfaceType(bool /*_inLibrary*/) const { return TypePointer(); }
private:
/// @returns a member list containing all members added to this type by `using for` directives.
@ -355,7 +355,7 @@ public:
u256 literalValue(Literal const* _literal) const override;
TypePointer encodingType() const override { return shared_from_this(); }
TypePointer interfaceType(bool) const override { return shared_from_this(); }
TypeResult interfaceType(bool) const override { return shared_from_this(); }
StateMutability stateMutability(void) const { return m_stateMutability; }
@ -395,7 +395,7 @@ public:
std::string toString(bool _short) const override;
TypePointer encodingType() const override { return shared_from_this(); }
TypePointer interfaceType(bool) const override { return shared_from_this(); }
TypeResult interfaceType(bool) const override { return shared_from_this(); }
unsigned numBits() const { return m_bits; }
bool isSigned() const { return m_modifier == Modifier::Signed; }
@ -437,7 +437,7 @@ public:
std::string toString(bool _short) const override;
TypePointer encodingType() const override { return shared_from_this(); }
TypePointer interfaceType(bool) const override { return shared_from_this(); }
TypeResult interfaceType(bool) const override { return shared_from_this(); }
/// Number of bits used for this type in total.
unsigned numBits() const { return m_totalBits; }
@ -583,7 +583,7 @@ public:
std::string toString(bool) const override { return "bytes" + dev::toString(m_bytes); }
MemberList::MemberMap nativeMembers(ContractDefinition const*) const override;
TypePointer encodingType() const override { return shared_from_this(); }
TypePointer interfaceType(bool) const override { return shared_from_this(); }
TypeResult interfaceType(bool) const override { return shared_from_this(); }
unsigned numBytes() const { return m_bytes; }
@ -609,7 +609,7 @@ public:
std::string toString(bool) const override { return "bool"; }
u256 literalValue(Literal const* _literal) const override;
TypePointer encodingType() const override { return shared_from_this(); }
TypePointer interfaceType(bool) const override { return shared_from_this(); }
TypeResult interfaceType(bool) const override { return shared_from_this(); }
};
/**
@ -716,7 +716,7 @@ public:
MemberList::MemberMap nativeMembers(ContractDefinition const* _currentScope) const override;
TypePointer encodingType() const override;
TypePointer decodingType() const override;
TypePointer interfaceType(bool _inLibrary) const override;
TypeResult interfaceType(bool _inLibrary) const override;
/// @returns true if this is valid to be stored in calldata
bool validForCalldata() const;
@ -749,8 +749,8 @@ private:
TypePointer m_baseType;
bool m_hasDynamicLength = true;
u256 m_length;
mutable boost::optional<TypePointer> m_interfaceType;
mutable boost::optional<TypePointer> m_interfaceType_library;
mutable boost::optional<TypeResult> m_interfaceType;
mutable boost::optional<TypeResult> m_interfaceType_library;
};
/**
@ -788,7 +788,7 @@ public:
return TypePointer{};
return std::make_shared<AddressType>(isPayable() ? StateMutability::Payable : StateMutability::NonPayable);
}
TypePointer interfaceType(bool _inLibrary) const override
TypeResult interfaceType(bool _inLibrary) const override
{
if (isSuper())
return TypePointer{};
@ -842,7 +842,17 @@ public:
{
return location() == DataLocation::Storage ? std::make_shared<IntegerType>(256) : shared_from_this();
}
TypePointer interfaceType(bool _inLibrary) const override;
TypeResult interfaceType(bool _inLibrary) const override;
bool recursive() const
{
if (m_recursive.is_initialized())
return m_recursive.get();
interfaceType(false);
return m_recursive.get();
}
TypePointer copyForLocation(DataLocation _location, bool _isPointer) const override;
@ -863,17 +873,12 @@ 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.
// Caches for interfaceType(bool)
mutable boost::optional<TypeResult> m_interfaceType;
mutable boost::optional<TypeResult> m_interfaceType_library;
mutable boost::optional<bool> m_recursive;
mutable boost::optional<TypePointer> m_interfaceType;
mutable boost::optional<TypePointer> m_interfaceType_library;
};
/**
@ -902,7 +907,7 @@ public:
{
return std::make_shared<IntegerType>(8 * int(storageBytes()));
}
TypePointer interfaceType(bool _inLibrary) const override
TypeResult interfaceType(bool _inLibrary) const override
{
return _inLibrary ? shared_from_this() : encodingType();
}
@ -1098,7 +1103,7 @@ public:
bool hasSimpleZeroValueInMemory() const override { return false; }
MemberList::MemberMap nativeMembers(ContractDefinition const* _currentScope) const override;
TypePointer encodingType() const override;
TypePointer interfaceType(bool _inLibrary) const override;
TypeResult interfaceType(bool _inLibrary) const override;
/// @returns TypePointer of a new FunctionType object. All input/return parameters are an
/// appropriate external types (i.e. the interfaceType()s) of input/return parameters of
@ -1223,10 +1228,7 @@ public:
{
return std::make_shared<IntegerType>(256);
}
TypePointer 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, ""); }

View File

@ -107,9 +107,9 @@ Json::Value ABI::generate(ContractDefinition const& _contractDef)
for (auto const& p: it->parameters())
{
auto type = p->annotation().type->interfaceType(false);
solAssert(type, "");
solAssert(type.get(), "");
Json::Value input;
auto param = formatType(p->name(), *type, false);
auto param = formatType(p->name(), *type.get(), false);
param["indexed"] = p->isIndexed();
params.append(param);
}
@ -173,8 +173,8 @@ Json::Value ABI::formatType(string const& _name, Type const& _type, bool _forLib
{
solAssert(member.type, "");
auto t = member.type->interfaceType(_forLibrary);
solAssert(t, "");
ret["components"].append(formatType(member.name, *t, _forLibrary));
solAssert(t.get(), "");
ret["components"].append(formatType(member.name, *t.get(), _forLibrary));
}
}
else

View File

@ -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.

View File

@ -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.

View File

@ -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.

View File

@ -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.

View File

@ -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.

View File

@ -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.

View File

@ -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.

View File

@ -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.

View File

@ -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.

View File

@ -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.

View File

@ -0,0 +1,9 @@
library Test {
struct MyStructName {
address addr;
MyStructName[] x;
}
function f(MyStructName storage s) public {}
}
// ----

View File

@ -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.

View File

@ -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.

View File

@ -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.

View File

@ -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.

View File

@ -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.

View File

@ -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.

View File

@ -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.