From 4e98cb3ed93e608845d3505a0a60ee49a37496d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 23 Jun 2012 09:23:14 +0200 Subject: [PATCH] Fixed #3907 (improve check: detect buffer overrun when using && or || in for loop) --- lib/checkbufferoverrun.cpp | 36 +++++++++++++++++++++++++++++------- test/testbufferoverrun.cpp | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 7 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 3a7a20c28..353cca078 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -283,29 +283,41 @@ static const Token *for_init(const Token *tok, unsigned int &varid, std::string /** Parse for condition */ -static bool for_condition(const Token * const tok2, unsigned int varid, std::string &min_value, std::string &max_value, bool &maxMinFlipped) +static bool for_condition(const Token *tok2, unsigned int varid, std::string &min_value, std::string &max_value, bool &maxMinFlipped) { - if (Token::Match(tok2, "%varid% < %num% ;", varid) || + if (Token::Match(tok2, "%varid% < %num% ;|&&|%oror%", varid) || Token::Match(tok2, "%varid% != %num% ; ++ %varid%", varid) || Token::Match(tok2, "%varid% != %num% ; %varid% ++", varid)) { maxMinFlipped = false; const MathLib::bigint value = MathLib::toLongNumber(tok2->strAt(2)); max_value = MathLib::toString(value - 1); - } else if (Token::Match(tok2, "%varid% <= %num% ;", varid)) { + } else if (Token::Match(tok2, "%varid% <= %num% ;|&&|%oror%", varid)) { maxMinFlipped = false; max_value = tok2->strAt(2); - } else if (Token::Match(tok2, "%num% < %varid% ;", varid) || + } else if (Token::Match(tok2, "%num% < %varid% ;|&&|%oror%", varid) || Token::Match(tok2, "%num% != %varid% ; ++ %varid%", varid) || Token::Match(tok2, "%num% != %varid% ; %varid% ++", varid)) { maxMinFlipped = true; const MathLib::bigint value = MathLib::toLongNumber(tok2->str()); max_value = min_value; min_value = MathLib::toString(value + 1); - } else if (Token::Match(tok2, "%num% <= %varid% ;", varid)) { + } else if (Token::Match(tok2, "%num% <= %varid% ;|&&|%oror%", varid)) { maxMinFlipped = true; max_value = min_value; min_value = tok2->str(); } else { + // parse condition + while (tok2 && tok2->str() != ";") { + if (tok2->str() == "(") + tok2 = tok2->link(); + else if (tok2->str() == ")") // unexpected ")" => break + break; + if (tok2->str() == "&&" || tok2->str() == "||") { + if (for_condition(tok2->next(), varid, min_value, max_value, maxMinFlipped)) + return true; + } + tok2 = tok2->next(); + } return false; } @@ -746,10 +758,20 @@ void CheckBufferOverrun::checkScopeForBody(const Token *tok, const ArrayInfo &ar if (MathLib::toLongNumber(max_counter_value) < size) condition_out_of_bounds = false; - if (!for3(tok2->tokAt(4), counter_varid, min_counter_value, max_counter_value, maxMinFlipped)) + // Goto the end of the condition + while (tok2 && tok2->str() != ";") { + if (tok2->str() == "(") + tok2 = tok2->link(); + else if (tok2->str() == ")") // unexpected ")" => break + break; + tok2 = tok2->next(); + } + if (!tok2 || tok2->str() != ";") + return; + if (!for3(tok2->next(), counter_varid, min_counter_value, max_counter_value, maxMinFlipped)) return; - if (Token::Match(tok2->tokAt(4), "%var% =|+=|-=") && MathLib::toLongNumber(max_counter_value) <= size) + if (Token::Match(tok2->next(), "%var% =|+=|-=") && MathLib::toLongNumber(max_counter_value) <= size) condition_out_of_bounds = false; // Goto the end parenthesis of the for-statement: "for (x; y; z)" .. diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 7ae2853e7..5c0850641 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -123,6 +123,7 @@ private: TEST_CASE(array_index_for); // FN: for,if TEST_CASE(array_index_for_neq); // #2211: Using != in condition TEST_CASE(array_index_for_question); // #2561: for, ?: + TEST_CASE(array_index_for_andand_oror); // FN: using && or || in the for loop condition TEST_CASE(array_index_vla_for); // #3221: access VLA inside for TEST_CASE(array_index_extern); // FP when using 'extern'. #1684 TEST_CASE(array_index_cast); // FP after cast. #2841 @@ -1688,6 +1689,43 @@ private: ASSERT_EQUALS("", errout.str()); } + void array_index_for_andand_oror() { // #3907 - using && or || + check("void f() {\n" + " char data[2];\n" + " int x;\n" + " for (x = 0; x < 10 && y; x++) {\n" + " data[x] = 0;\n" + " }" + "}"); + ASSERT_EQUALS("[test.cpp:5]: (error) Buffer access out-of-bounds: data\n", errout.str()); + + check("void f() {\n" + " char data[2];\n" + " int x;\n" + " for (x = 0; x < 10 || y; x++) {\n" + " data[x] = 0;\n" + " }" + "}"); + ASSERT_EQUALS("[test.cpp:5]: (error) Buffer access out-of-bounds: data\n", errout.str()); + + check("void f() {\n" + " char data[2];\n" + " int x;\n" + " for (x = 0; x <= 10 && y; x++) {\n" + " data[x] = 0;\n" + " }" + "}"); + ASSERT_EQUALS("[test.cpp:5]: (error) Buffer access out-of-bounds: data\n", errout.str()); + + check("void f() {\n" + " char data[2];\n" + " int x;\n" + " for (x = 0; y && x <= 10; x++) {\n" + " data[x] = 0;\n" + " }" + "}"); + ASSERT_EQUALS("[test.cpp:5]: (error) Buffer access out-of-bounds: data\n", errout.str()); + } void array_index_for_break() { check("void f() {\n"