From 97cb571ba49b81bd20b840e20f27c2cf55730d81 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 21 Aug 2017 12:29:04 +0200 Subject: [PATCH 1/3] Tests for recursion in JULIA. --- test/libjulia/Parser.cpp | 12 ++++++++++++ test/libsolidity/InlineAssembly.cpp | 14 ++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/test/libjulia/Parser.cpp b/test/libjulia/Parser.cpp index e1bf5a3a3..bfe764612 100644 --- a/test/libjulia/Parser.cpp +++ b/test/libjulia/Parser.cpp @@ -237,6 +237,18 @@ BOOST_AUTO_TEST_CASE(builtin_types) BOOST_CHECK(successParse("{ let x:s256 := 1:s256 }")); } +BOOST_AUTO_TEST_CASE(recursion_depth) +{ + string input; + for (size_t i = 0; i < 20000; i++) + input += "{"; + input += "let x:u256 := 0:u256"; + for (size_t i = 0; i < 20000; i++) + input += "}"; + + CHECK_ERROR(input, ParserError, "recursio"); +} + BOOST_AUTO_TEST_SUITE_END() } diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp index 8e1c304ac..0debc66d8 100644 --- a/test/libsolidity/InlineAssembly.cpp +++ b/test/libsolidity/InlineAssembly.cpp @@ -400,6 +400,20 @@ BOOST_AUTO_TEST_CASE(instruction_too_many_arguments) CHECK_PARSE_ERROR("{ mul(1, 2, 3) }", ParserError, "Expected ')' (\"mul\" expects 2 arguments)"); } +BOOST_AUTO_TEST_CASE(recursion_depth) +{ + string input; + for (size_t i = 0; i < 20000; i++) + input += "{"; + input += "let x := 0"; + for (size_t i = 0; i < 20000; i++) + input += "}"; + + CHECK_PARSE_ERROR(input, ParserError, "recursion"); +} + + + BOOST_AUTO_TEST_SUITE_END() BOOST_AUTO_TEST_SUITE(Printing) From 692e4c57e83607f21d0c1b1b735585b3b63564f3 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 21 Aug 2017 12:33:29 +0200 Subject: [PATCH 2/3] Check recursion depth in assembly parser. --- Changelog.md | 1 + libsolidity/inlineasm/AsmParser.cpp | 11 ++++++++++ libsolidity/parsing/Parser.cpp | 32 ----------------------------- libsolidity/parsing/Parser.h | 7 ------- libsolidity/parsing/ParserBase.cpp | 13 ++++++++++++ libsolidity/parsing/ParserBase.h | 20 ++++++++++++++++++ test/libjulia/Parser.cpp | 2 +- 7 files changed, 46 insertions(+), 40 deletions(-) diff --git a/Changelog.md b/Changelog.md index 3574455b7..962aae2e5 100644 --- a/Changelog.md +++ b/Changelog.md @@ -11,6 +11,7 @@ Features: Bugfixes: * Assembly Parser: Be more strict about number literals. + * Assembly Parser: Limit maximum recursion depth. * Parser: Enforce commas between array and tuple elements. * Parser: Limit maximum recursion depth. * Type Checker: Crash fix related to ``using``. diff --git a/libsolidity/inlineasm/AsmParser.cpp b/libsolidity/inlineasm/AsmParser.cpp index 1dcc42b80..d84fe999c 100644 --- a/libsolidity/inlineasm/AsmParser.cpp +++ b/libsolidity/inlineasm/AsmParser.cpp @@ -36,6 +36,7 @@ using namespace dev::solidity::assembly; shared_ptr Parser::parse(std::shared_ptr const& _scanner) { + m_recursionDepth = 0; try { m_scanner = _scanner; @@ -51,6 +52,7 @@ shared_ptr Parser::parse(std::shared_ptr const& _scann assembly::Block Parser::parseBlock() { + RecursionGuard recursionGuard(*this); assembly::Block block = createWithLocation(); expectToken(Token::LBrace); while (currentToken() != Token::RBrace) @@ -62,6 +64,7 @@ assembly::Block Parser::parseBlock() assembly::Statement Parser::parseStatement() { + RecursionGuard recursionGuard(*this); switch (currentToken()) { case Token::Let: @@ -158,6 +161,7 @@ assembly::Statement Parser::parseStatement() assembly::Case Parser::parseCase() { + RecursionGuard recursionGuard(*this); assembly::Case _case = createWithLocation(); if (m_scanner->currentToken() == Token::Default) m_scanner->next(); @@ -178,6 +182,7 @@ assembly::Case Parser::parseCase() assembly::ForLoop Parser::parseForLoop() { + RecursionGuard recursionGuard(*this); ForLoop forLoop = createWithLocation(); expectToken(Token::For); forLoop.pre = parseBlock(); @@ -192,6 +197,7 @@ assembly::ForLoop Parser::parseForLoop() assembly::Statement Parser::parseExpression() { + RecursionGuard recursionGuard(*this); Statement operation = parseElementaryOperation(true); if (operation.type() == typeid(Instruction)) { @@ -254,6 +260,7 @@ std::map const& Parser::instructionNames() assembly::Statement Parser::parseElementaryOperation(bool _onlySinglePusher) { + RecursionGuard recursionGuard(*this); Statement ret; switch (currentToken()) { @@ -342,6 +349,7 @@ assembly::Statement Parser::parseElementaryOperation(bool _onlySinglePusher) assembly::VariableDeclaration Parser::parseVariableDeclaration() { + RecursionGuard recursionGuard(*this); VariableDeclaration varDecl = createWithLocation(); expectToken(Token::Let); while (true) @@ -366,6 +374,7 @@ assembly::VariableDeclaration Parser::parseVariableDeclaration() assembly::FunctionDefinition Parser::parseFunctionDefinition() { + RecursionGuard recursionGuard(*this); FunctionDefinition funDef = createWithLocation(); expectToken(Token::Function); funDef.name = expectAsmIdentifier(); @@ -397,6 +406,7 @@ assembly::FunctionDefinition Parser::parseFunctionDefinition() assembly::Statement Parser::parseCall(assembly::Statement&& _instruction) { + RecursionGuard recursionGuard(*this); if (_instruction.type() == typeid(Instruction)) { solAssert(!m_julia, "Instructions are invalid in JULIA"); @@ -479,6 +489,7 @@ assembly::Statement Parser::parseCall(assembly::Statement&& _instruction) TypedName Parser::parseTypedName() { + RecursionGuard recursionGuard(*this); TypedName typedName = createWithLocation(); typedName.name = expectAsmIdentifier(); if (m_julia) diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index 7455cbca9..1ab38f823 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -64,25 +64,6 @@ private: SourceLocation m_location; }; -/// Utility class that creates an error and throws an exception if the -/// recursion depth is too deep. -class Parser::RecursionGuard -{ -public: - explicit RecursionGuard(Parser& _parser): - m_parser(_parser) - { - m_parser.increaseRecursionDepth(); - } - ~RecursionGuard() - { - m_parser.decreaseRecursionDepth(); - } - -private: - Parser& m_parser; -}; - ASTPointer Parser::parse(shared_ptr const& _scanner) { try @@ -1542,19 +1523,6 @@ ASTPointer Parser::createEmptyParameterList() return nodeFactory.createNode(vector>()); } -void Parser::increaseRecursionDepth() -{ - m_recursionDepth++; - if (m_recursionDepth >= 4096) - fatalParserError("Maximum recursion depth reached during parsing."); -} - -void Parser::decreaseRecursionDepth() -{ - solAssert(m_recursionDepth > 0, ""); - m_recursionDepth--; -} - string Parser::currentTokenName() { Token::Value token = m_scanner->currentToken(); diff --git a/libsolidity/parsing/Parser.h b/libsolidity/parsing/Parser.h index 0f74880cf..cfdfea7e2 100644 --- a/libsolidity/parsing/Parser.h +++ b/libsolidity/parsing/Parser.h @@ -41,7 +41,6 @@ public: private: class ASTNodeFactory; - class RecursionGuard; struct VarDeclParserOptions { @@ -165,14 +164,8 @@ private: /// Creates an empty ParameterList at the current location (used if parameters can be omitted). ASTPointer createEmptyParameterList(); - /// Increases the recursion depth and throws an exception if it is too deep. - void increaseRecursionDepth(); - void decreaseRecursionDepth(); - /// Flag that signifies whether '_' is parsed as a PlaceholderStatement or a regular identifier. bool m_insideModifier = false; - /// Current recursion depth during parsing. - size_t m_recursionDepth = 0; }; } diff --git a/libsolidity/parsing/ParserBase.cpp b/libsolidity/parsing/ParserBase.cpp index 5657c2c02..db988739d 100644 --- a/libsolidity/parsing/ParserBase.cpp +++ b/libsolidity/parsing/ParserBase.cpp @@ -101,6 +101,19 @@ void ParserBase::expectToken(Token::Value _value) m_scanner->next(); } +void ParserBase::increaseRecursionDepth() +{ + m_recursionDepth++; + if (m_recursionDepth >= 4096) + fatalParserError("Maximum recursion depth reached during parsing."); +} + +void ParserBase::decreaseRecursionDepth() +{ + solAssert(m_recursionDepth > 0, ""); + m_recursionDepth--; +} + void ParserBase::parserError(string const& _description) { m_errorReporter.parserError(SourceLocation(position(), position(), sourceName()), _description); diff --git a/libsolidity/parsing/ParserBase.h b/libsolidity/parsing/ParserBase.h index 48733fc1c..fd0de0d10 100644 --- a/libsolidity/parsing/ParserBase.h +++ b/libsolidity/parsing/ParserBase.h @@ -41,6 +41,20 @@ public: std::shared_ptr const& sourceName() const; protected: + /// Utility class that creates an error and throws an exception if the + /// recursion depth is too deep. + class RecursionGuard + { + public: + explicit RecursionGuard(ParserBase& _parser): m_parser(_parser) + { + m_parser.increaseRecursionDepth(); + } + ~RecursionGuard() { m_parser.decreaseRecursionDepth(); } + private: + ParserBase& m_parser; + }; + /// Start position of the current token int position() const; /// End position of the current token @@ -56,6 +70,10 @@ protected: Token::Value advance(); ///@} + /// Increases the recursion depth and throws an exception if it is too deep. + void increaseRecursionDepth(); + void decreaseRecursionDepth(); + /// Creates a @ref ParserError and annotates it with the current position and the /// given @a _description. void parserError(std::string const& _description); @@ -67,6 +85,8 @@ protected: std::shared_ptr m_scanner; /// The reference to the list of errors and warning to add errors/warnings during parsing ErrorReporter& m_errorReporter; + /// Current recursion depth during parsing. + size_t m_recursionDepth = 0; }; } diff --git a/test/libjulia/Parser.cpp b/test/libjulia/Parser.cpp index bfe764612..510703708 100644 --- a/test/libjulia/Parser.cpp +++ b/test/libjulia/Parser.cpp @@ -246,7 +246,7 @@ BOOST_AUTO_TEST_CASE(recursion_depth) for (size_t i = 0; i < 20000; i++) input += "}"; - CHECK_ERROR(input, ParserError, "recursio"); + CHECK_ERROR(input, ParserError, "recursion"); } BOOST_AUTO_TEST_SUITE_END() From 628b54ce351dd5de8ec34aa8c2c23f2fd0a77d90 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 22 Aug 2017 12:32:15 +0200 Subject: [PATCH 3/3] Reduce max recursion depth. --- libsolidity/parsing/ParserBase.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libsolidity/parsing/ParserBase.cpp b/libsolidity/parsing/ParserBase.cpp index db988739d..fe95b0fee 100644 --- a/libsolidity/parsing/ParserBase.cpp +++ b/libsolidity/parsing/ParserBase.cpp @@ -104,7 +104,7 @@ void ParserBase::expectToken(Token::Value _value) void ParserBase::increaseRecursionDepth() { m_recursionDepth++; - if (m_recursionDepth >= 4096) + if (m_recursionDepth >= 3000) fatalParserError("Maximum recursion depth reached during parsing."); }