From c8004a8d314702f14acbf516c2586f05906f393d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Tue, 25 Mar 2014 18:22:22 +0100 Subject: [PATCH] Buffer overruns: Use ValueFlow to detect negative index --- lib/checkbufferoverrun.cpp | 17 +++++++++++++++++ lib/valueflow.cpp | 12 +++++++++++- test/testbufferoverrun.cpp | 38 +++++++++++++++++++++----------------- test/testvalueflow.cpp | 7 +++++++ 4 files changed, 56 insertions(+), 18 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 85a3b33db..39f05f2ec 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -1183,6 +1183,23 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo else if (Token::Match(tok, "%varid% [", arrayInfo.declarationId())) { // Look for errors first for (int warn = 0; warn == 0 || warn == 1; ++warn) { + // Negative index.. + for (const Token *tok2 = tok->next(); tok2 && tok2->str() == "["; tok2 = tok2->link()->next()) { + const Token *index = tok2->astOperand2(); + std::list::const_iterator it; + const ValueFlow::Value *val = nullptr; + for (it = index->values.begin(); it != index->values.end(); ++it) { + if (it->intvalue < 0) { + val = &*it; + if (val->condition == nullptr) + break; + } + } + if (val && !val->condition) + negativeIndexError(index, val->intvalue); + } + + // Index out of bounds.. std::vector indexes; unsigned int valuevarid = 0; for (const Token *tok2 = tok->next(); indexes.size() < arrayInfo.num().size() && Token::Match(tok2, "["); tok2 = tok2->link()->next()) { diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index f9315ac15..f145a25ab 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -663,6 +663,11 @@ static void execute(const Token *expr, if (var == programMemory->end()) *error = true; else { + if (var->second == 0 && + expr->str() == "--" && + expr->astOperand1()->variable() && + expr->astOperand1()->variable()->typeStartToken()->isUnsigned()) + *error = true; // overflow *result = var->second + (expr->str() == "++" ? 1 : -1); var->second = *result; } @@ -800,8 +805,13 @@ static void valueFlowForLoopSimplify(Token * const bodyStart, const unsigned int while (parent) { const Token * const p = parent; parent = parent->astParent(); - if (parent && parent->str() == "?" && parent->astOperand2() == p) + if (parent && parent->str() == ":") break; + if (parent && parent->str() == "?") { + if (parent->astOperand2() != p) + parent = NULL; + break; + } } if (parent) { if (settings->debugwarnings) diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index cc55729a8..0f878228c 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -969,48 +969,48 @@ private: " a[-1] = 0;\n" // negative index " a[" + charMaxPlusOne.str() + "] = 0;\n" // 128/256 > CHAR_MAX "}\n").c_str()); - ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a["+charMaxPlusOne.str()+"]' accessed at index "+charMaxPlusOne.str()+", which is out of bounds.\n" - "[test.cpp:3]: (error) Array index -1 is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n" + "[test.cpp:4]: (error) Array 'a["+charMaxPlusOne.str()+"]' accessed at index "+charMaxPlusOne.str()+", which is out of bounds.\n", errout.str()); check("void f(signed char n) {\n" " int a[n];\n" // n <= SCHAR_MAX " a[-1] = 0;\n" // negative index " a[128] = 0;\n" // 128 > SCHAR_MAX "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[128]' accessed at index 128, which is out of bounds.\n" - "[test.cpp:3]: (error) Array index -1 is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n" + "[test.cpp:4]: (error) Array 'a[128]' accessed at index 128, which is out of bounds.\n", errout.str()); check("void f(unsigned char n) {\n" " int a[n];\n" // n <= UCHAR_MAX " a[-1] = 0;\n" // negative index " a[256] = 0;\n" // 256 > UCHAR_MAX "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[256]' accessed at index 256, which is out of bounds.\n" - "[test.cpp:3]: (error) Array index -1 is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n" + "[test.cpp:4]: (error) Array 'a[256]' accessed at index 256, which is out of bounds.\n", errout.str()); check("void f(short n) {\n" " int a[n];\n" // n <= SHRT_MAX " a[-1] = 0;\n" // negative index " a[32768] = 0;\n" // 32768 > SHRT_MAX "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[32768]' accessed at index 32768, which is out of bounds.\n" - "[test.cpp:3]: (error) Array index -1 is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n" + "[test.cpp:4]: (error) Array 'a[32768]' accessed at index 32768, which is out of bounds.\n", errout.str()); check("void f(unsigned short n) {\n" " int a[n];\n" // n <= USHRT_MAX " a[-1] = 0;\n" // negative index " a[65536] = 0;\n" // 65536 > USHRT_MAX "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[65536]' accessed at index 65536, which is out of bounds.\n" - "[test.cpp:3]: (error) Array index -1 is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n" + "[test.cpp:4]: (error) Array 'a[65536]' accessed at index 65536, which is out of bounds.\n", errout.str()); check("void f(signed short n) {\n" " int a[n];\n" // n <= SHRT_MAX " a[-1] = 0;\n" // negative index " a[32768] = 0;\n" // 32768 > SHRT_MAX "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[32768]' accessed at index 32768, which is out of bounds.\n" - "[test.cpp:3]: (error) Array index -1 is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n" + "[test.cpp:4]: (error) Array 'a[32768]' accessed at index 32768, which is out of bounds.\n", errout.str()); check("void f(int n) {\n" " int a[n];\n" // n <= INT_MAX @@ -1066,7 +1066,8 @@ private: " for (int i = 0; i < 10; i++)\n" " a[i-1] = a[i];\n" "}"); - ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n" + "[test.cpp:5]: (error) Array index -1 is out of bounds.\n", errout.str()); } void array_index_28() { @@ -1789,7 +1790,8 @@ private: " val[i+1] = val[i];\n" " }\n" "}"); - TODO_ASSERT_EQUALS("[test.cpp:5]: (error) Array 'val[5]' index -1 out of bounds.\n", "", errout.str()); + ASSERT_EQUALS("[test.cpp:5]: (error) Array index -9994 is out of bounds.\n" + "[test.cpp:5]: (error) Array index -9995 is out of bounds.\n", errout.str()); } @@ -1872,7 +1874,7 @@ private: " a[i - 1] = 0;\n" " }\n" "}"); - TODO_ASSERT_EQUALS("[test.cpp:7]: (error) Array 'a[2]' accessed at index -1, which is out of bounds", "", errout.str()); + ASSERT_EQUALS("[test.cpp:7]: (error) Array index -1 is out of bounds.\n", errout.str()); } void array_index_for() { @@ -1939,7 +1941,8 @@ private: " some_condition ? 0 : a[i-1];\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n" + "[test.cpp:4]: (error) Array index -1 is out of bounds.\n", errout.str()); check("void f() {\n" " int a[10];\n" @@ -1948,7 +1951,8 @@ private: " a[i-1] = 0;\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n" + "[test.cpp:5]: (error) Array index -1 is out of bounds.\n", errout.str()); } void array_index_for_varid0() { // #4228: No varid for counter variable diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index a2e965c99..86a5e57d6 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -655,6 +655,13 @@ private: ASSERT_EQUALS(true, testValueOfX(code, 3U, 0)); ASSERT_EQUALS(true, testValueOfX(code, 3U, 9)); ASSERT_EQUALS(false, testValueOfX(code, 4U, 9)); + + code = "void f() {\n" + " for (int x = 0; x < 10; x++)\n" + " x==0 ?\n" + " 0 : a[x];\n" + "}"; + ASSERT_EQUALS(false, testValueOfX(code, 4U, 0)); } void valueFlowSubFunction() {