From: Jelte Jansen Date: Tue, 27 Nov 2012 21:23:53 +0000 (+0100) Subject: [master] Fix overflow catching in number lexer state X-Git-Tag: bind10-1.0.0-beta-release~46^2~24 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=019d55a3b50fffc8694f982ccc457dc1c61a04a2;p=thirdparty%2Fkea.git [master] Fix overflow catching in number lexer state --- diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc index 8364f80b3a..a68c36e9c9 100644 --- a/src/lib/dns/master_lexer.cc +++ b/src/lib/dns/master_lexer.cc @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -210,7 +211,8 @@ const char* const error_text[] = { "unbalanced parentheses", // UNBALANCED_PAREN "unexpected end of input", // UNEXPECTED_END "unbalanced quotes", // UNBALANCED_QUOTES - "no token produced" // NO_TOKEN_PRODUCED + "no token produced", // NO_TOKEN_PRODUCED + "number out of range" // NUMBER_RANGE }; const size_t error_text_max_count = sizeof(error_text) / sizeof(error_text[0]); } // end unnamed namespace @@ -449,8 +451,6 @@ QString::handle(MasterLexer& lexer) const { void Number::handle(MasterLexer& lexer) const { MasterLexer::Token& token = getLexerImpl(lexer)->token_; - // Do we want to support octal and/or hex here? - const int base = 10; // It may yet turn out to be a string, so we first // collect all the data @@ -465,10 +465,17 @@ Number::handle(MasterLexer& lexer) const { if (getLexerImpl(lexer)->isTokenEnd(c, escaped)) { getLexerImpl(lexer)->source_->ungetChar(); if (digits_only) { - // Close the string for strtoul + // Close the string for lexical_cast data.push_back('\0'); - token = MasterLexer::Token(strtoul(&data.at(0), - NULL, base)); + try { + const uint32_t number32 = + boost::lexical_cast(&data.at(0)); + token = MasterLexer::Token(number32); + } catch (const boost::bad_lexical_cast&) { + // Since we already know we have only digits, + // range should be the only possible problem. + token = Token(Token::NUMBER_RANGE); + } } else { token = MasterLexer::Token(&data.at(0), data.size()); diff --git a/src/lib/dns/master_lexer.h b/src/lib/dns/master_lexer.h index 431e0ce9b1..2717c87c01 100644 --- a/src/lib/dns/master_lexer.h +++ b/src/lib/dns/master_lexer.h @@ -292,6 +292,7 @@ public: UNBALANCED_QUOTES, ///< Unbalanced quotations detected NO_TOKEN_PRODUCED, ///< No token was produced. This means programmer /// error and should never get out of the lexer. + NUMBER_RANGE, ///< Number was out of range MAX_ERROR_CODE ///< Max integer corresponding to valid error codes. /// (excluding this one). Mainly for internal use. }; diff --git a/src/lib/dns/tests/master_lexer_state_unittest.cc b/src/lib/dns/tests/master_lexer_state_unittest.cc index aea28d4026..b5c1c96c8e 100644 --- a/src/lib/dns/tests/master_lexer_state_unittest.cc +++ b/src/lib/dns/tests/master_lexer_state_unittest.cc @@ -457,12 +457,9 @@ TEST_F(MasterLexerStateTest, basicNumbers) { ss << "1 "; ss << "12345 "; ss << "4294967295 "; // 2^32-1 - ss << "4294967296 "; // 2^32 (this overflows to 0, we - // can consider failing on it, but - // this is what bind9 does as well) - ss << "4294967297 "; // 2^32+1 (this overflows to 1, see - // above) - ss << "1000000000000000000 "; // overflows to 2808348672 + ss << "4294967296 "; // Out of range + ss << "340282366920938463463374607431768211456 "; + // Very much out of range (2^128) ss << "005 "; // Leading zeroes are ignored ss << "42;asdf\n"; // Number with comment ss << "37"; // Simple number again, here to make @@ -484,19 +481,17 @@ TEST_F(MasterLexerStateTest, basicNumbers) { EXPECT_EQ(&s_number, State::start(lexer, common_options)); s_number.handle(lexer); - EXPECT_EQ(4294967295u, s_number.getToken(lexer).getNumber()); + EXPECT_EQ(4294967295, s_number.getToken(lexer).getNumber()); EXPECT_EQ(&s_number, State::start(lexer, common_options)); s_number.handle(lexer); - EXPECT_EQ(0, s_number.getToken(lexer).getNumber()); - - EXPECT_EQ(&s_number, State::start(lexer, common_options)); - s_number.handle(lexer); - EXPECT_EQ(1, s_number.getToken(lexer).getNumber()); + EXPECT_EQ(Token::NUMBER_RANGE, + s_number.getToken(lexer).getErrorCode()); EXPECT_EQ(&s_number, State::start(lexer, common_options)); s_number.handle(lexer); - EXPECT_EQ(2808348672u, s_number.getToken(lexer).getNumber()); + EXPECT_EQ(Token::NUMBER_RANGE, + s_number.getToken(lexer).getErrorCode()); EXPECT_EQ(&s_number, State::start(lexer, common_options)); s_number.handle(lexer); diff --git a/src/lib/dns/tests/master_lexer_token_unittest.cc b/src/lib/dns/tests/master_lexer_token_unittest.cc index 5f887f9df7..0c13658d38 100644 --- a/src/lib/dns/tests/master_lexer_token_unittest.cc +++ b/src/lib/dns/tests/master_lexer_token_unittest.cc @@ -145,15 +145,18 @@ TEST_F(MasterLexerTokenTest, errors) { EXPECT_EQ("no token produced", MasterLexer::Token(MasterLexer::Token::NO_TOKEN_PRODUCED). getErrorText()); + EXPECT_EQ("number out of range", + MasterLexer::Token(MasterLexer::Token::NUMBER_RANGE). + getErrorText()); // getErrorCode/Text() isn't allowed for non number types EXPECT_THROW(token_num.getErrorCode(), isc::InvalidOperation); EXPECT_THROW(token_num.getErrorText(), isc::InvalidOperation); - // Only the pre-defined error code is accepted. Hardcoding '5' (max code + // Only the pre-defined error code is accepted. Hardcoding '6' (max code // + 1) is intentional; it'd be actually better if we notice it when we // update the enum list (which shouldn't happen too often). - EXPECT_THROW(MasterLexer::Token(MasterLexer::Token::ErrorCode(5)), + EXPECT_THROW(MasterLexer::Token(MasterLexer::Token::ErrorCode(6)), isc::InvalidParameter); // Check the coexistence of "from number" and "from error-code"