diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 958e0b575..97d167ed2 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -2179,3 +2179,35 @@ void CheckBufferOverrun::executionPaths() + +void CheckBufferOverrun::arrayIndexThenCheck() +{ + if (!_settings->_checkCodingStyle) + return; + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + if (Token::Match(tok, "%var% [ %var% ]")) + { + const std::string arrayName(tok->str()); + const std::string indexName(tok->strAt(2)); + + // skip array index.. + tok = tok->tokAt(4); + while (tok->str() == "[") + tok = tok->link()->next(); + + // skip comparison + if (Token::Match(tok, "==|!=|<|<=|>|>= %any% &&")) + tok = tok->tokAt(2); + + // check if array index is ok + if (Token::Match(tok, ("&& " + indexName + " <|<=").c_str())) + arrayIndexThenCheckError(tok, indexName); + } + } +} + +void CheckBufferOverrun::arrayIndexThenCheckError(const Token *tok, const std::string &indexName) +{ + reportError(tok, Severity::style, "arrayIndexThenCheck", "array index " + indexName + " is used before bounds check"); +} diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index f004fa014..77739b252 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -64,6 +64,7 @@ public: CheckBufferOverrun checkBufferOverrun(tokenizer, settings, errorLogger); checkBufferOverrun.bufferOverrun(); checkBufferOverrun.negativeIndex(); + checkBufferOverrun.arrayIndexThenCheck(); /** ExecutionPath checking.. */ checkBufferOverrun.executionPaths(); @@ -72,6 +73,9 @@ public: /** @brief %Check for buffer overruns */ void bufferOverrun(); + /** @brief Using array index before bounds check */ + void arrayIndexThenCheck(); + /** @brief %Check for buffer overruns by inspecting execution paths */ void executionPaths(); @@ -216,6 +220,7 @@ public: void negativeIndexError(const Token *tok, MathLib::bigint index); void cmdLineArgsError(const Token *tok); void pointerOutOfBounds(const Token *tok, const std::string &object); // UB when result of calculation is out of bounds + void arrayIndexThenCheckError(const Token *tok, const std::string &indexName); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) { @@ -229,6 +234,7 @@ public: c.negativeIndexError(0, -1); c.cmdLineArgsError(0); c.pointerOutOfBounds(0, "array"); + c.arrayIndexThenCheckError(0, "index"); } std::string myName() const diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 83f53e992..3aa68370e 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -60,6 +60,7 @@ private: CheckBufferOverrun checkBufferOverrun(&tokenizer, &settings, this); checkBufferOverrun.bufferOverrun(); checkBufferOverrun.negativeIndex(); + checkBufferOverrun.arrayIndexThenCheck(); } void run() @@ -217,6 +218,9 @@ private: TEST_CASE(getErrorMessages); TEST_CASE(unknownMacroNoDecl); // #2638 - not variable declaration: 'AAA a[0] = 0;' + + // Access array and then check if the used index is within bounds + TEST_CASE(arrayIndexThenCheck); } @@ -3015,6 +3019,21 @@ private: "}"); ASSERT_EQUALS("", errout.str()); } + + void arrayIndexThenCheck() + { + check("void f(const char s[]) {\n" + " if (s[i] == 'x' && i < y) {\n" + " }" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) array index i is used before bounds check\n", errout.str()); + + check("void f(const char s[]) {\n" + " for (i = 0; s[i] == 'x' && i < y; ++i) {\n" + " }" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) array index i is used before bounds check\n", errout.str()); + } }; REGISTER_TEST(TestBufferOverrun)