From 4669b06ab448b61350bff23db12da1a8e9fff5fa Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 15 Jan 2019 22:49:15 +0100 Subject: [PATCH] Warn if type(..).runtimeCode is used with assembly in the constructor. --- libsolidity/analysis/StaticAnalyzer.cpp | 62 +++++++++++++++++++ libsolidity/analysis/StaticAnalyzer.h | 9 ++- .../metaTypes/runtimeCodeWarningAssembly.sol | 17 +++++ 3 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 test/libsolidity/syntaxTests/metaTypes/runtimeCodeWarningAssembly.sol diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp index aaaa4f9f5..11ed6a4fe 100644 --- a/libsolidity/analysis/StaticAnalyzer.cpp +++ b/libsolidity/analysis/StaticAnalyzer.cpp @@ -32,6 +32,56 @@ using namespace dev; using namespace langutil; using namespace dev::solidity; +/** + * Helper class that determines whether a contract's constructor uses inline assembly. + */ +class dev::solidity::ConstructorUsesAssembly +{ +public: + /// @returns true if and only if the contract's or any of its bases' constructors + /// use inline assembly. + bool check(ContractDefinition const& _contract) + { + for (auto const* base: _contract.annotation().linearizedBaseContracts) + if (checkInternal(*base)) + return true; + return false; + } + + +private: + class Checker: public ASTConstVisitor + { + public: + Checker(FunctionDefinition const& _f) { _f.accept(*this); } + bool visit(InlineAssembly const&) override { assemblySeen = true; return false; } + bool assemblySeen = false; + }; + + bool checkInternal(ContractDefinition const& _contract) + { + if (!m_usesAssembly.count(&_contract)) + { + bool usesAssembly = false; + if (_contract.constructor()) + usesAssembly = Checker{*_contract.constructor()}.assemblySeen; + m_usesAssembly[&_contract] = usesAssembly; + } + return m_usesAssembly[&_contract]; + } + + map m_usesAssembly; +}; + +StaticAnalyzer::StaticAnalyzer(ErrorReporter& _errorReporter): + m_errorReporter(_errorReporter) +{ +} + +StaticAnalyzer::~StaticAnalyzer() +{ +} + bool StaticAnalyzer::analyze(SourceUnit const& _sourceUnit) { _sourceUnit.accept(*this); @@ -152,6 +202,18 @@ bool StaticAnalyzer::visit(MemberAccess const& _memberAccess) _memberAccess.location(), "\"block.blockhash()\" has been deprecated in favor of \"blockhash()\"" ); + else if (type->kind() == MagicType::Kind::MetaType && _memberAccess.memberName() == "runtimeCode") + { + if (!m_constructorUsesAssembly) + m_constructorUsesAssembly = make_unique(); + ContractType const& contract = dynamic_cast(*type->typeArgument()); + if (m_constructorUsesAssembly->check(contract.contractDefinition())) + m_errorReporter.warning( + _memberAccess.location(), + "The constructor of the contract (or its base) uses inline assembly. " + "Because of that, it might be that the deployed bytecode is different from type(...).runtimeCode." + ); + } } if (_memberAccess.memberName() == "callcode") diff --git a/libsolidity/analysis/StaticAnalyzer.h b/libsolidity/analysis/StaticAnalyzer.h index ff33fa3a4..3daf83b31 100644 --- a/libsolidity/analysis/StaticAnalyzer.h +++ b/libsolidity/analysis/StaticAnalyzer.h @@ -38,6 +38,8 @@ namespace dev namespace solidity { +class ConstructorUsesAssembly; + /** * The module that performs static analysis on the AST. @@ -49,7 +51,8 @@ class StaticAnalyzer: private ASTConstVisitor { public: /// @param _errorReporter provides the error logging functionality. - explicit StaticAnalyzer(langutil::ErrorReporter& _errorReporter): m_errorReporter(_errorReporter) {} + explicit StaticAnalyzer(langutil::ErrorReporter& _errorReporter); + ~StaticAnalyzer(); /// Performs static analysis on the given source unit and all of its sub-nodes. /// @returns true iff all checks passed. Note even if all checks passed, errors() can still contain warnings @@ -85,6 +88,10 @@ private: /// when traversing. std::map, int> m_localVarUseCount; + /// Cache that holds information about whether a contract's constructor + /// uses inline assembly. + std::unique_ptr m_constructorUsesAssembly; + FunctionDefinition const* m_currentFunction = nullptr; /// Flag that indicates a constructor. diff --git a/test/libsolidity/syntaxTests/metaTypes/runtimeCodeWarningAssembly.sol b/test/libsolidity/syntaxTests/metaTypes/runtimeCodeWarningAssembly.sol new file mode 100644 index 000000000..ec8d97842 --- /dev/null +++ b/test/libsolidity/syntaxTests/metaTypes/runtimeCodeWarningAssembly.sol @@ -0,0 +1,17 @@ +contract Test { + function f() public pure returns (uint) { + return type(C).runtimeCode.length + + type(D).runtimeCode.length + + type(C).creationCode.length + + type(D).creationCode.length; + } +} +contract C { + constructor() public { assembly {} } +} +contract D is C { + constructor() public {} +} +// ---- +// Warning: (77-96): The constructor of the contract (or its base) uses inline assembly. Because of that, it might be that the deployed bytecode is different from type(...).runtimeCode. +// Warning: (118-137): The constructor of the contract (or its base) uses inline assembly. Because of that, it might be that the deployed bytecode is different from type(...).runtimeCode.