From 19e9e42dd7fc8ba327a2736cc091285d37faff80 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Sun, 17 Mar 2019 14:22:26 +0100 Subject: [PATCH] Library: Enhance minsize configuration and allow simple values. (#1736) Some POSIX and Windows functions require buffers of at least some specific size. This is now possible to configure via for example this minsize configuration: ``. The range for valid buffer size values is 1 to LLONG_MAX (9223372036854775807) --- cfg/cppcheck-cfg.rng | 51 +++++++++++++++++++++---------- cfg/posix.cfg | 4 +-- lib/checkbufferoverrun.cpp | 2 ++ lib/library.cpp | 46 +++++++++++++++++++--------- lib/library.h | 5 +-- test/cfg/posix.c | 62 ++++++++++++++++++++++++++++++++++++++ test/testlibrary.cpp | 13 +++++++- 7 files changed, 148 insertions(+), 35 deletions(-) diff --git a/cfg/cppcheck-cfg.rng b/cfg/cppcheck-cfg.rng index 1339fbba7..b5e8b5196 100644 --- a/cfg/cppcheck-cfg.rng +++ b/cfg/cppcheck-cfg.rng @@ -196,24 +196,36 @@ - - - - strlen - argvalue - sizeof - mul - - - - - - - + + + + + strlen + argvalue + sizeof + mul + + + - - + + + + + + + + + + value + + + + + + + @@ -495,4 +507,11 @@ empty + + + + 1 + 9223372036854775807 + + diff --git a/cfg/posix.cfg b/cfg/posix.cfg index 947123ea8..2a1f5afe2 100644 --- a/cfg/posix.cfg +++ b/cfg/posix.cfg @@ -2776,7 +2776,7 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s - + @@ -2793,7 +2793,7 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s - + diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 8ef4d9626..54e724610 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -372,6 +372,8 @@ static bool checkBufferSize(const Token *ftok, const Library::ArgumentChecks::Mi if (arg && arg2 && arg->hasKnownIntValue() && arg2->hasKnownIntValue()) return (arg->getKnownIntValue() * arg2->getKnownIntValue()) <= bufferSize; break; + case Library::ArgumentChecks::MinSize::Type::VALUE: + return minsize.value <= bufferSize; case Library::ArgumentChecks::MinSize::Type::NONE: break; }; diff --git a/lib/library.cpp b/lib/library.cpp index 56512aae6..37691d7c3 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -662,24 +662,42 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co type = ArgumentChecks::MinSize::Type::SIZEOF; else if (strcmp(typeattr,"mul")==0) type = ArgumentChecks::MinSize::Type::MUL; + else if (strcmp(typeattr,"value")==0) + type = ArgumentChecks::MinSize::Type::VALUE; else return Error(BAD_ATTRIBUTE_VALUE, typeattr); - const char *argattr = argnode->Attribute("arg"); - if (!argattr) - return Error(MISSING_ATTRIBUTE, "arg"); - if (strlen(argattr) != 1 || argattr[0]<'0' || argattr[0]>'9') - return Error(BAD_ATTRIBUTE_VALUE, argattr); + if (type == ArgumentChecks::MinSize::Type::VALUE) { + const char *valueattr = argnode->Attribute("value"); + if (!valueattr) + return Error(MISSING_ATTRIBUTE, "value"); + long long minsizevalue = 0; + try { + minsizevalue = MathLib::toLongNumber(valueattr); + } catch (const InternalError&) { + return Error(BAD_ATTRIBUTE_VALUE, valueattr); + } + if (minsizevalue <= 0) + return Error(BAD_ATTRIBUTE_VALUE, valueattr); + ac.minsizes.emplace_back(type, 0); + ac.minsizes.back().value = minsizevalue; + } else { + const char *argattr = argnode->Attribute("arg"); + if (!argattr) + return Error(MISSING_ATTRIBUTE, "arg"); + if (strlen(argattr) != 1 || argattr[0]<'0' || argattr[0]>'9') + return Error(BAD_ATTRIBUTE_VALUE, argattr); - ac.minsizes.reserve(type == ArgumentChecks::MinSize::Type::MUL ? 2 : 1); - ac.minsizes.emplace_back(type,argattr[0]-'0'); - if (type == ArgumentChecks::MinSize::Type::MUL) { - const char *arg2attr = argnode->Attribute("arg2"); - if (!arg2attr) - return Error(MISSING_ATTRIBUTE, "arg2"); - if (strlen(arg2attr) != 1 || arg2attr[0]<'0' || arg2attr[0]>'9') - return Error(BAD_ATTRIBUTE_VALUE, arg2attr); - ac.minsizes.back().arg2 = arg2attr[0] - '0'; + ac.minsizes.reserve(type == ArgumentChecks::MinSize::Type::MUL ? 2 : 1); + ac.minsizes.emplace_back(type, argattr[0] - '0'); + if (type == ArgumentChecks::MinSize::Type::MUL) { + const char *arg2attr = argnode->Attribute("arg2"); + if (!arg2attr) + return Error(MISSING_ATTRIBUTE, "arg2"); + if (strlen(arg2attr) != 1 || arg2attr[0]<'0' || arg2attr[0]>'9') + return Error(BAD_ATTRIBUTE_VALUE, arg2attr); + ac.minsizes.back().arg2 = arg2attr[0] - '0'; + } } } diff --git a/lib/library.h b/lib/library.h index 508f970b9..9198072b8 100644 --- a/lib/library.h +++ b/lib/library.h @@ -254,11 +254,12 @@ public: class MinSize { public: - enum Type { NONE, STRLEN, ARGVALUE, SIZEOF, MUL }; - MinSize(Type t, int a) : type(t), arg(a), arg2(0) {} + enum Type { NONE, STRLEN, ARGVALUE, SIZEOF, MUL, VALUE }; + MinSize(Type t, int a) : type(t), arg(a), arg2(0), value(0) {} Type type; int arg; int arg2; + long long value; }; std::vector minsizes; diff --git a/test/cfg/posix.c b/test/cfg/posix.c index 148bfeb26..5eed324be 100644 --- a/test/cfg/posix.c +++ b/test/cfg/posix.c @@ -350,3 +350,65 @@ void dl(const char* libname, const char* func) dlclose(uninit); // cppcheck-suppress resourceLeak } + +void asctime_r_test(struct tm * tm, char * bufSizeUnknown) +{ + struct tm tm_uninit_data; + struct tm * tm_uninit_pointer; + char bufSize5[5]; + char bufSize25[25]; + char bufSize26[26]; + char bufSize100[100]; + + // cppcheck-suppress asctime_rCalled + // cppcheck-suppress bufferAccessOutOfBounds + asctime_r(tm, bufSize5); + // cppcheck-suppress asctime_rCalled + // cppcheck-suppress bufferAccessOutOfBounds + asctime_r(tm, bufSize25); + // cppcheck-suppress asctime_rCalled + asctime_r(tm, bufSize26); + // cppcheck-suppress asctime_rCalled + asctime_r(tm, bufSize100); + + // cppcheck-suppress asctime_rCalled + // cppcheck-suppress uninitvar + asctime_r(&tm_uninit_data, bufSize100); + // cppcheck-suppress asctime_rCalled + // cppcheck-suppress uninitvar + asctime_r(tm_uninit_pointer, bufSize100); + + // cppcheck-suppress asctime_rCalled + asctime_r(tm, bufSizeUnknown); +} + +void ctime_r_test(time_t * timep, char * bufSizeUnknown) +{ + time_t time_t_uninit_data; + time_t * time_t_uninit_pointer; + char bufSize5[5]; + char bufSize25[25]; + char bufSize26[26]; + char bufSize100[100]; + + // cppcheck-suppress ctime_rCalled + // cppcheck-suppress bufferAccessOutOfBounds + ctime_r(timep, bufSize5); + // cppcheck-suppress ctime_rCalled + // cppcheck-suppress bufferAccessOutOfBounds + ctime_r(timep, bufSize25); + // cppcheck-suppress ctime_rCalled + ctime_r(timep, bufSize26); + // cppcheck-suppress ctime_rCalled + ctime_r(timep, bufSize100); + + // cppcheck-suppress ctime_rCalled + // cppcheck-suppress uninitvar + ctime_r(&time_t_uninit_data, bufSize100); + // cppcheck-suppress ctime_rCalled + // cppcheck-suppress uninitvar + ctime_r(time_t_uninit_pointer, bufSize100); + + // cppcheck-suppress ctime_rCalled + ctime_r(timep, bufSizeUnknown); +} diff --git a/test/testlibrary.cpp b/test/testlibrary.cpp index 490ce9aae..0e8c5e468 100644 --- a/test/testlibrary.cpp +++ b/test/testlibrary.cpp @@ -448,6 +448,7 @@ private: " \n" " \n" " \n" + " \n" " \n" ""; @@ -455,7 +456,7 @@ private: ASSERT_EQUALS(true, Library::OK == (readLibrary(library, xmldata)).errorcode); TokenList tokenList(nullptr); - std::istringstream istr("foo(a,b,c);"); + std::istringstream istr("foo(a,b,c,d);"); tokenList.createTokens(istr); tokenList.front()->next()->astOperand1(tokenList.front()); @@ -478,6 +479,16 @@ private: ASSERT_EQUALS(Library::ArgumentChecks::MinSize::ARGVALUE, m.type); ASSERT_EQUALS(3, m.arg); } + + // arg4: type=value + minsizes = library.argminsizes(tokenList.front(), 4); + ASSERT_EQUALS(true, minsizes != nullptr); + ASSERT_EQUALS(1U, minsizes ? minsizes->size() : 1U); + if (minsizes && minsizes->size() == 1U) { + const Library::ArgumentChecks::MinSize &m = minsizes->front(); + ASSERT_EQUALS(Library::ArgumentChecks::MinSize::VALUE, m.type); + ASSERT_EQUALS(500, m.value); + } } void function_namespace() const {