From 6aa400fd803d2d4533aad9d40f06dfd60860ac6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 26 Dec 2010 21:23:28 +0100 Subject: [PATCH] Buffer overrun: UB when pointer arithmetic result points out of bounds. Ticket #1774 --- lib/checkbufferoverrun.cpp | 16 ++++++++++++++++ lib/checkbufferoverrun.h | 2 ++ test/testbufferoverrun.cpp | 16 ++++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index aadae8aaa..2c93f6055 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -125,6 +125,12 @@ 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) +{ + reportError(tok, Severity::portability, "pointerOutOfBounds", "Undefined behaviour: pointer arithmetic result does not point into or just past the end of the array\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"); +} + void CheckBufferOverrun::sizeArgumentAsChar(const Token *tok) { if (_settings && !_settings->_checkCodingStyle) @@ -1078,6 +1084,16 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo if (static_cast(n) > total_size) outOfBounds(tok->tokAt(4), "snprintf size"); } + + // undefined behaviour: result of pointer arithmetic is out of bounds + if (_settings->_checkCodingStyle && Token::Match(tok, "= %varid% + %num% ;", arrayInfo.varid)) + { + const MathLib::bigint index = MathLib::toLongNumber(tok->strAt(3)); + if (index < 0 || index > arrayInfo.num[0]) + { + pointerOutOfBounds(tok->next()); + } + } } } diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index 7ba954f65..942afbe69 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -186,6 +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 getErrorMessages() { @@ -197,6 +198,7 @@ public: terminateStrncpyError(0); negativeIndexError(0, -1); cmdLineArgsError(0); + pointerOutOfBounds(0); } std::string name() const diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index f908a0b63..333da1b61 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -131,6 +131,13 @@ private: TEST_CASE(buffer_overrun_15); // ticket #1787 TEST_CASE(buffer_overrun_16); + // It is undefined behaviour to point out of bounds of an array + // the address beyond the last element is in bounds + // char a[10]; + // char *p1 = a + 10; // OK + // char *p2 = a + 11 // UB + TEST_CASE(pointer_out_of_bounds_1); + TEST_CASE(sprintf1); TEST_CASE(sprintf2); TEST_CASE(sprintf3); @@ -1800,6 +1807,15 @@ private: ASSERT_EQUALS("", errout.str()); } + void pointer_out_of_bounds_1() + { + check("void f() {\n" + " char a[10];\n" + " char *p = a + 100;\n" + "}"); + 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 sprintf1() { check("void f()\n"