diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index e86c4f55a..bfeb4c4f7 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -125,9 +125,9 @@ void CheckBufferOverrun::outOfBounds(const Token *tok, const std::string &what) reportError(tok, Severity::error, "outOfBounds", what + " is out of bounds"); } -void CheckBufferOverrun::pointerOutOfBounds(const Token *tok) +void CheckBufferOverrun::pointerOutOfBounds(const Token *tok, const std::string &object) { - reportError(tok, Severity::portability, "pointerOutOfBounds", "Undefined behaviour: pointer arithmetic result does not point into or just past the end of the array\n" + reportError(tok, Severity::portability, "pointerOutOfBounds", "Undefined behaviour: pointer arithmetic result does not point into or just past the end of the " + object + "\n" "Undefined behaviour: Using pointer arithmetic so that the result does not point into or just past the end of the same object. Further information: https://www.securecoding.cert.org/confluence/display/seccode/ARR30-C.+Do+not+form+or+use+out+of+bounds+pointers+or+array+subscripts"); } @@ -726,6 +726,10 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vectornext()) { @@ -902,6 +906,21 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vectorstrAt(3)); + if (index > size && _settings->_checkCodingStyle) + pointerOutOfBounds(tok->next(), "buffer"); + if (index >= size && Token::Match(tok->tokAt(-2), "[;{}] %varid% =", varid)) + pointerIsOutOfBounds = true; + } + + if (pointerIsOutOfBounds && Token::Match(tok, "[;{}=] * %varid% [;=]", varid)) + { + outOfBounds(tok->tokAt(2), tok->strAt(2)); + } } } @@ -1156,7 +1175,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo const MathLib::bigint index = MathLib::toLongNumber(tok->strAt(3)); if (index < 0 || index > arrayInfo.num[0]) { - pointerOutOfBounds(tok->next()); + pointerOutOfBounds(tok->next(), "array"); } } } diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index b8e6444dd..81006cc99 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -186,7 +186,7 @@ public: void terminateStrncpyError(const Token *tok); void negativeIndexError(const Token *tok, MathLib::bigint index); void cmdLineArgsError(const Token *tok); - void pointerOutOfBounds(const Token *tok); // UB when result of calculation is out of bounds + void pointerOutOfBounds(const Token *tok, const std::string &object); // UB when result of calculation is out of bounds void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) { @@ -199,7 +199,7 @@ public: c.terminateStrncpyError(0); c.negativeIndexError(0, -1); c.cmdLineArgsError(0); - c.pointerOutOfBounds(0); + c.pointerOutOfBounds(0, "array"); } std::string name() const diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 1f53b6d3b..5fd547f4c 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -139,6 +139,7 @@ private: // char *p1 = a + 10; // OK // char *p2 = a + 11 // UB TEST_CASE(pointer_out_of_bounds_1); + TEST_CASE(pointer_out_of_bounds_2); TEST_CASE(sprintf1); TEST_CASE(sprintf2); @@ -1883,6 +1884,33 @@ private: ASSERT_EQUALS("[test.cpp:3]: (portability) Undefined behaviour: pointer arithmetic result does not point into or just past the end of the array\n", errout.str()); } + void pointer_out_of_bounds_2() + { + check("void f() {\n" + " char *p = malloc(10);\n" + " p += 100;\n" + " free(p);" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (portability) Undefined behaviour: pointer arithmetic result does not point into or just past the end of the buffer\n", errout.str()); + + check("void f() {\n" + " char *p = malloc(10);\n" + " p += 10;\n" + " *p = 0;\n" + " free(p);" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) p is out of bounds\n", errout.str()); + + check("void f() {\n" + " char *p = malloc(10);\n" + " p += 10;\n" + " p = p - 1\n" + " *p = 0;\n" + " free(p);" + "}"); + ASSERT_EQUALS("", errout.str()); + } + void sprintf1() { check("void f()\n"