diff --git a/cfg/std.cfg b/cfg/std.cfg index 385a69a67..d81325015 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -73,13 +73,6 @@ - - false - - - 0: - - false false @@ -135,7 +128,7 @@ false - + @@ -184,7 +177,7 @@ false - + 0: @@ -196,7 +189,7 @@ false - + @@ -294,35 +287,35 @@ false - + 0: 0: false - - + + 0: false - - + + 0: false - - + + 0: false - + 0: @@ -505,7 +498,7 @@ false - + @@ -535,14 +528,14 @@ false - + 0: false - + 0: diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 9b97b579b..3b5fb855b 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -264,87 +264,65 @@ static bool bailoutIfSwitch(const Token *tok, const unsigned int varid) } //--------------------------------------------------------------------------- -void CheckBufferOverrun::checkFunctionParameter(const Token &tok, unsigned int par, const ArrayInfo &arrayInfo, const std::list& callstack) +void CheckBufferOverrun::checkFunctionParameter(const Token &ftok, unsigned int par, const ArrayInfo &arrayInfo, const std::list& callstack) { - // total_size : which parameter in function call takes the total size? - std::map total_size; + const std::list * const minsizes = _settings->library.argminsizes(ftok.str(),par); - if (!(Token::simpleMatch(tok.previous(), ".") || Token::Match(tok.tokAt(-2), "!!std ::"))) { - - total_size["fgets"] = 2; // The second argument for fgets can't exceed the total size of the array - total_size["memcmp"] = 3; - total_size["memcpy"] = 3; - total_size["memmove"] = 3; - total_size["memchr"] = 3; - - if (par == 1) { - // reading from array - // if it is zero terminated properly there won't be buffer overruns - total_size["strncat"] = 3; - total_size["strncpy"] = 3; - total_size["memset"] = 3; - total_size["fread"] = 1001; // parameter 2 * parameter 3 - total_size["fwrite"] = 1001; // parameter 2 * parameter 3 - } - - else if (par == 2) { - if (_settings->standards.posix) { - total_size["read"] = 3; - total_size["pread"] = 3; - total_size["write"] = 3; - total_size["recv"] = 3; - total_size["recvfrom"] = 3; - total_size["send"] = 3; - total_size["sendto"] = 3; - } - } - } - - std::map::const_iterator it = total_size.find(tok.str()); - if (it != total_size.end()) { + if (minsizes && !minsizes->empty() && (!(Token::simpleMatch(ftok.previous(), ".") || Token::Match(ftok.tokAt(-2), "!!std ::")))) { if (arrayInfo.element_size() == 0) return; - // arg : the index of the "wanted" argument in the function call. - const unsigned int arg = it->second; + MathLib::bigint arraySize = arrayInfo.element_size(); + for (unsigned int i = 0; i < arrayInfo.num().size(); ++i) + arraySize *= arrayInfo.num(i); - // Parse function call. When a ',' is seen, arg is decremented. - // if arg becomes 1 then the current function parameter is the wanted parameter. - // if arg becomes 1001 then multiply current and next argument. - const Token *tok2 = tok.tokAt(2)->nextArgument(); - if (arg == 3) - tok2 = tok2->nextArgument(); - if ((arg == 2 || arg == 3) && tok2) { - if (Token::Match(tok2, "%num% ,|)")) { - const MathLib::bigint sz = MathLib::toLongNumber(tok2->str()); - MathLib::bigint elements = 1; - for (unsigned int i = 0; i < arrayInfo.num().size(); ++i) - elements *= arrayInfo.num(i); - if (sz < 0 || sz > int(elements * arrayInfo.element_size())) { - bufferOverrunError(callstack, arrayInfo.varname()); + bool error = true; + for (std::list::const_iterator minsize = minsizes->begin(); minsize != minsizes->end(); ++minsize) { + if (!error) + break; + error = false; + const Token *argtok = ftok.tokAt(2); + for (int argnum = 1; argtok && argnum < minsize->arg; argnum++) + argtok = argtok->nextArgument(); + if (!argtok) + break; + switch (minsize->type) { + case Library::ArgumentChecks::MinSize::Type::ARGVALUE: + if (Token::Match(argtok, "%num% ,|)")) { + const MathLib::bigint sz = MathLib::toLongNumber(argtok->str()); + if (sz > arraySize) + error = true; + } else if (argtok->type() == Token::eChar && Token::Match(argtok->next(), ",|)")) + sizeArgumentAsCharError(argtok); + break; + case Library::ArgumentChecks::MinSize::Type::MUL: + // TODO: handle arbitrary arg2 + if (minsize->arg2 == minsize->arg+1 && Token::Match(argtok, "%num% , %num% ,|)")) { + const MathLib::bigint sz = MathLib::toLongNumber(argtok->str()) * MathLib::toLongNumber(argtok->strAt(2)); + if (sz > arraySize) + error = true; } - } - - else if (Token::Match(tok2->next(), ",|)") && tok2->type() == Token::eChar) { - sizeArgumentAsCharError(tok2); - } - } else if (arg == 1001) { // special code. This parameter multiplied with the next must not exceed total_size - if (Token::Match(tok2, "%num% , %num% ,|)")) { - const MathLib::bigint sz = MathLib::toLongNumber(MathLib::multiply(tok2->str(), tok2->strAt(2))); - MathLib::bigint elements = 1; - for (unsigned int i = 0; i < arrayInfo.num().size(); ++i) - elements *= arrayInfo.num(i); - if (sz < 0 || sz > int(elements * arrayInfo.element_size())) { - bufferOverrunError(&tok, arrayInfo.varname()); - } - } + break; + case Library::ArgumentChecks::MinSize::Type::STRLEN: + if (argtok->type() == Token::eString && Token::getStrLength(argtok) >= arraySize) + error = true; + break; + case Library::ArgumentChecks::MinSize::Type::SIZEOF: + if (argtok->type() == Token::eString && Token::getStrLength(argtok) >= arraySize) + error = true; + break; + case Library::ArgumentChecks::MinSize::Type::NONE: + break; + }; } + if (error) + bufferOverrunError(callstack, arrayInfo.varname()); } // Calling a user function? // only 1-dimensional arrays can be checked currently else if (arrayInfo.num().size() == 1) { - const Function* const func = tok.function(); + const Function* const func = ftok.function(); if (func && func->hasBody) { // Get corresponding parameter.. @@ -361,38 +339,38 @@ void CheckBufferOverrun::checkFunctionParameter(const Token &tok, unsigned int p return; // Check the parameter usage in the function scope.. - for (const Token* ftok = func->functionScope->classStart; ftok != func->functionScope->classEnd; ftok = ftok->next()) { - if (Token::Match(ftok, "if|for|switch|while (")) { + for (const Token* ftok2 = func->functionScope->classStart; ftok2 != func->functionScope->classEnd; ftok2 = ftok2->next()) { + if (Token::Match(ftok2, "if|for|switch|while (")) { // bailout if there is buffer usage.. - if (bailoutIfSwitch(ftok, parameter->declarationId())) { + if (bailoutIfSwitch(ftok2, parameter->declarationId())) { break; } // no bailout is needed. skip the if-block else { // goto end of if block.. - ftok = ftok->next()->link()->next()->link(); - if (Token::simpleMatch(ftok, "} else {")) - ftok = ftok->linkAt(2); - if (!ftok) + ftok2 = ftok2->linkAt(1)->linkAt(1); + if (Token::simpleMatch(ftok2, "} else {")) + ftok2 = ftok2->linkAt(2); + if (!ftok2) break; continue; } } - if (ftok->str() == "}") + if (ftok2->str() == "}") break; - if (ftok->varId() == parameter->declarationId()) { - if (Token::Match(ftok->previous(), "-- %var%") || - Token::Match(ftok, "%var% --")) + if (ftok2->varId() == parameter->declarationId()) { + if (Token::Match(ftok2->previous(), "-- %var%") || + Token::Match(ftok2, "%var% --")) break; - if (Token::Match(ftok->previous(), ";|{|}|%op% %var% [ %num% ]")) { - const MathLib::bigint index = MathLib::toLongNumber(ftok->strAt(2)); + if (Token::Match(ftok2->previous(), ";|{|}|%op% %var% [ %num% ]")) { + const MathLib::bigint index = MathLib::toLongNumber(ftok2->strAt(2)); if (index >= 0 && arrayInfo.num(0) > 0 && index >= arrayInfo.num(0)) { std::list callstack2(callstack); - callstack2.push_back(ftok); + callstack2.push_back(ftok2); std::vector indexes; indexes.push_back(index); @@ -403,10 +381,10 @@ void CheckBufferOverrun::checkFunctionParameter(const Token &tok, unsigned int p } // Calling function.. - if (Token::Match(ftok, "%var% (")) { + if (Token::Match(ftok2, "%var% (")) { ArrayInfo ai(arrayInfo); ai.declarationId(parameter->declarationId()); - checkFunctionCall(ftok, ai, callstack); + checkFunctionCall(ftok2, ai, callstack); } } } @@ -414,7 +392,7 @@ void CheckBufferOverrun::checkFunctionParameter(const Token &tok, unsigned int p // Check 'float x[10]' arguments in declaration if (_settings->isEnabled("warning")) { - const Function* const func = tok.function(); + const Function* const func = ftok.function(); // If argument is '%type% a[num]' then check bounds against num if (func) { @@ -439,7 +417,7 @@ void CheckBufferOverrun::checkFunctionParameter(const Token &tok, unsigned int p arraysize *= arrayInfo.num(i); if (Token::Match(tok2, "[,)]") && arraysize > 0 && argsize > arraysize) - argumentSizeError(&tok, tok.str(), arrayInfo.varname()); + argumentSizeError(&ftok, ftok.str(), arrayInfo.varname()); } } } diff --git a/lib/library.cpp b/lib/library.cpp index fb02004b2..dcd9dfe95 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -169,6 +169,7 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) bool formatstr = false; bool strz = false; std::string valid; + std::list minsizes; for (const tinyxml2::XMLElement *argnode = functionnode->FirstChildElement(); argnode; argnode = argnode->NextSiblingElement()) { if (strcmp(argnode->Name(), "not-bool") == 0) notbool = true; @@ -206,6 +207,40 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) valid = argnode->GetText(); } + else if (strcmp(argnode->Name(), "minsize") == 0) { + const char *typeattr = argnode->Attribute("type"); + if (!typeattr) + return Error(MISSING_ATTRIBUTE, "type"); + + ArgumentChecks::MinSize::Type type; + if (strcmp(typeattr,"strlen")==0) + type = ArgumentChecks::MinSize::Type::STRLEN; + else if (strcmp(typeattr,"argvalue")==0) + type = ArgumentChecks::MinSize::Type::ARGVALUE; + else if (strcmp(typeattr,"sizeof")==0) + type = ArgumentChecks::MinSize::Type::SIZEOF; + else if (strcmp(typeattr,"mul")==0) + type = ArgumentChecks::MinSize::Type::MUL; + 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); + + minsizes.push_back(ArgumentChecks::MinSize(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); + minsizes.back().arg2 = arg2attr[0] - '0'; + } + } + else return Error(BAD_ATTRIBUTE, argnode->Name()); } @@ -214,7 +249,8 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) argumentChecks[name][nr].notuninit = notuninit; argumentChecks[name][nr].formatstr = formatstr; argumentChecks[name][nr].strz = strz; - argumentChecks[name][nr].valid = valid; + argumentChecks[name][nr].valid.swap(valid); + argumentChecks[name][nr].minsizes.swap(minsizes); } else if (strcmp(functionnode->Name(), "ignorefunction") == 0) { _ignorefunction.insert(name); } else if (strcmp(functionnode->Name(), "formatstr") == 0) { diff --git a/lib/library.h b/lib/library.h index 41a591a57..f050df0c4 100644 --- a/lib/library.h +++ b/lib/library.h @@ -152,6 +152,16 @@ public: bool formatstr; bool strz; std::string valid; + + class MinSize { + public: + enum Type {NONE,STRLEN,ARGVALUE,SIZEOF,MUL}; + MinSize(Type t, int a) : type(t), arg(a), arg2(0) {} + Type type; + int arg; + int arg2; + }; + std::list minsizes; }; // function name, argument nr => argument data @@ -189,6 +199,24 @@ public: return arg ? arg->valid : emptyString; } + bool hasminsize(const std::string &functionName) const { + std::map >::const_iterator it1; + it1 = argumentChecks.find(functionName); + if (it1 == argumentChecks.end()) + return false; + std::map::const_iterator it2; + for (it2 = it1->second.begin(); it2 != it1->second.end(); ++it2) { + if (!it2->second.minsizes.empty()) + return true; + } + return false; + } + + const std::list *argminsizes(const std::string &functionName, int argnr) const { + const ArgumentChecks *arg = getarg(functionName, argnr); + return arg ? &arg->minsizes : nullptr; + } + bool markupFile(const std::string &path) const { return _markupExtensions.find(Path::getFilenameExtensionInLowerCase(path)) != _markupExtensions.end(); } diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 577fa8053..35099ca81 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -72,6 +72,31 @@ private: checkBufferOverrun.writeOutsideBufferSize(); } + void checkstd(const char code[], const char filename[] = "test.cpp") { + static bool init; + static Settings settings; + if (!init) { + init = true; + LOAD_LIB_2(settings.library, "std.cfg"); + settings.addEnabled("warning"); + } + + Tokenizer tokenizer(&settings, this); + std::istringstream istr(code); + tokenizer.tokenize(istr, filename); + + // Clear the error buffer.. + errout.str(""); + + // Check for buffer overruns.. + CheckBufferOverrun checkBufferOverrun(&tokenizer, &settings, this); + checkBufferOverrun.bufferOverrun(); + checkBufferOverrun.bufferOverrun2(); + checkBufferOverrun.arrayIndexThenCheck(); + checkBufferOverrun.writeOutsideBufferSize(); + } + + void run() { TEST_CASE(noerr1); TEST_CASE(noerr2); @@ -696,11 +721,11 @@ private: ASSERT_EQUALS("", errout.str()); // #3168 - check("void a(char *p) { memset(p,0,100); }\n" - "void b() {\n" - " char buf[10];\n" - " a(buf);" - "}"); + checkstd("void a(char *p) { memset(p,0,100); }\n" + "void b() {\n" + " char buf[10];\n" + " a(buf);" + "}"); ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1]: (error) Buffer is accessed out of bounds: buf\n", errout.str()); } @@ -2071,14 +2096,7 @@ private: ASSERT_EQUALS("", errout.str()); } - void buffer_overrun_1_standard_functions() { - check("void f()\n" - "{\n" - " char str[3];\n" - " strcpy(str, \"abc\");\n" - "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str\n", errout.str()); - + void buffer_overrun_1_posix_functions() { check("void f(int fd)\n" "{\n" " char str[3];\n" @@ -2114,50 +2132,6 @@ private: "}", false, "test.cpp", false); ASSERT_EQUALS("", errout.str()); - check("void f()\n" - "{\n" - " char str[3];\n" - " fgets(str, 3, stdin);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f()\n" - "{\n" - " char str[3];\n" - " fgets(str, 4, stdin);\n" - "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str\n", errout.str()); - - // fread - check("void f(FILE* fd)\n" - "{\n" - "char str[3];\n" - "fread(str,1,4,fd);\n" - "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str\n", errout.str()); - - check("void f(FILE* fd)\n" - "{\n" - "char str[3];\n" - "fread(str,1,3,fd);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - // fwrite - check("void f(FILE* fd)\n" - "{\n" - "char str[3];\n" - "fwrite(str,1,4,fd);\n" - "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str\n", errout.str()); - - check("void f(FILE* fd)\n" - "{\n" - "char str[3];\n" - "fwrite(str,1,3,fd);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - check("void f()\n" "{\n" "char str[3];\n" @@ -2185,14 +2159,61 @@ private: "sendto(s, str, 4, 0, 0x0, 0x0);\n" "}"); ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str\n", errout.str()); + } + + void buffer_overrun_1_standard_functions() { + check("void f()\n" + "{\n" + " char str[3];\n" + " strcpy(str, \"abc\");\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str\n", errout.str()); + + checkstd("void f() {\n" + " char str[3];\n" + " fgets(str, 3, stdin);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + checkstd("void f() {\n" + " char str[3];\n" + " fgets(str, 4, stdin);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: str\n", errout.str()); + + // fread + checkstd("void f(FILE* fd) {\n" + " char str[3];\n" + " fread(str,1,4,fd);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: str\n", errout.str()); + + checkstd("void f(FILE* fd) {\n" + " char str[3];\n" + " fread(str,1,3,fd);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // fwrite + checkstd("void f(FILE* fd) {\n" + " char str[3];\n" + " fwrite(str,1,4,fd);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: str\n", errout.str()); + + checkstd("void f(FILE* fd) {\n" + " char str[3];\n" + " fwrite(str,1,3,fd);\n" + "}"); + ASSERT_EQUALS("", errout.str()); // #4968 - not standard function - check("void f() {\n" - " char str[3];\n" - " foo.memset(str, 0, 100);\n" - " foo::memset(str, 0, 100);\n" - " std::memset(str, 0, 100);\n" - "}"); + checkstd("void f() {\n" + " char str[3];\n" + " foo.memset(str, 0, 100);\n" + " foo::memset(str, 0, 100);\n" + " std::memset(str, 0, 100);\n" + "}"); ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: str\n", errout.str()); } @@ -2423,92 +2444,92 @@ private: void buffer_overrun_13() { // ticket #836 - check("void f() {\n" - " char a[10];\n" - " memset(a+5, 0, 10);\n" - "}"); + checkstd("void f() {\n" + " char a[10];\n" + " memset(a+5, 0, 10);\n" + "}"); ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: a\n", errout.str()); - check("void f() {\n" - " char a[10];\n" - " memmove(a, a+5, 10);\n" - "}"); + checkstd("void f() {\n" + " char a[10];\n" + " memmove(a, a+5, 10);\n" + "}"); ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: a\n", errout.str()); } void buffer_overrun_14() { - check("void f(char *a) {\n" - " char *b = new char[strlen(a)];\n" - " strcpy(b, a);\n" - " return b;\n" - "}"); + checkstd("void f(char *a) {\n" + " char *b = new char[strlen(a)];\n" + " strcpy(b, a);\n" + " return b;\n" + "}"); ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", errout.str()); - check("void f(char *a) {\n" - " char *b = new char[strlen(a) + 1];\n" - " strcpy(b, a);\n" - " return b;\n" - "}"); + checkstd("void f(char *a) {\n" + " char *b = new char[strlen(a) + 1];\n" + " strcpy(b, a);\n" + " return b;\n" + "}"); ASSERT_EQUALS("", errout.str()); - check("void f(char *a) {\n" - " char *b = new char[strlen(a)];\n" - " a[0] = '\\0';\n" - " strcpy(b, a);\n" - " return b;\n" - "}"); + checkstd("void f(char *a) {\n" + " char *b = new char[strlen(a)];\n" + " a[0] = '\\0';\n" + " strcpy(b, a);\n" + " return b;\n" + "}"); ASSERT_EQUALS("", errout.str()); - check("void f(char *a) {\n" - " char *b = malloc(strlen(a));\n" - " b = realloc(b, 10000);\n" - " strcpy(b, a);\n" - " return b;\n" - "}"); + checkstd("void f(char *a) {\n" + " char *b = malloc(strlen(a));\n" + " b = realloc(b, 10000);\n" + " strcpy(b, a);\n" + " return b;\n" + "}"); ASSERT_EQUALS("", errout.str()); - check("void f(char *a) {\n" - " char *b = malloc(strlen(a));\n" - " strcpy(b, a);\n" - " return b;\n" - "}"); + checkstd("void f(char *a) {\n" + " char *b = malloc(strlen(a));\n" + " strcpy(b, a);\n" + " return b;\n" + "}"); ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", errout.str()); - check("void f(char *a) {\n" - " char *b = malloc(strlen(a));\n" - " {\n" - " strcpy(b, a);\n" - " }\n" - " return b;\n" - "}"); + checkstd("void f(char *a) {\n" + " char *b = malloc(strlen(a));\n" + " {\n" + " strcpy(b, a);\n" + " }\n" + " return b;\n" + "}"); ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds.\n", errout.str()); - check("void f(char *a) {\n" - " char *b = malloc(strlen(a) + 1);\n" - " strcpy(b, a);\n" - " return b;\n" - "}"); + checkstd("void f(char *a) {\n" + " char *b = malloc(strlen(a) + 1);\n" + " strcpy(b, a);\n" + " return b;\n" + "}"); ASSERT_EQUALS("", errout.str()); - check("void f(char *a, char *c) {\n" - " char *b = realloc(c, strlen(a));\n" - " strcpy(b, a);\n" - " return b;\n" - "}"); + checkstd("void f(char *a, char *c) {\n" + " char *b = realloc(c, strlen(a));\n" + " strcpy(b, a);\n" + " return b;\n" + "}"); ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", errout.str()); - check("void f(char *a, char *c) {\n" - " char *b = realloc(c, strlen(a) + 1);\n" - " strcpy(b, a);\n" - " return b;\n" - "}"); + checkstd("void f(char *a, char *c) {\n" + " char *b = realloc(c, strlen(a) + 1);\n" + " strcpy(b, a);\n" + " return b;\n" + "}"); ASSERT_EQUALS("", errout.str()); - check("void f(char *a) {\n" - " char *b = malloc(strlen(a));\n" - " sprintf(b, \"%s\", a);\n" - " return b;\n" - "}"); + checkstd("void f(char *a) {\n" + " char *b = malloc(strlen(a));\n" + " sprintf(b, \"%s\", a);\n" + " return b;\n" + "}"); ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", errout.str()); } @@ -2610,42 +2631,40 @@ private: } void buffer_overrun_22() { // ticket #3124 - check("class A {\n" - "public:\n" - " char b[5][6];\n" - "};\n" - "int main() {\n" - " A a;\n" - " memset(a.b, 0, 5 * 6);\n" - "}"); - + checkstd("class A {\n" + "public:\n" + " char b[5][6];\n" + "};\n" + "int main() {\n" + " A a;\n" + " memset(a.b, 0, 5 * 6);\n" + "}"); ASSERT_EQUALS("", errout.str()); - check("class A {\n" - "public:\n" - " char b[5][6];\n" - "};\n" - "int main() {\n" - " A a;\n" - " memset(a.b, 0, 6 * 6);\n" - "}"); - + checkstd("class A {\n" + "public:\n" + " char b[5][6];\n" + "};\n" + "int main() {\n" + " A a;\n" + " memset(a.b, 0, 6 * 6);\n" + "}"); ASSERT_EQUALS("[test.cpp:7]: (error) Buffer is accessed out of bounds: a.b\n", errout.str()); } void buffer_overrun_23() { // ticket #3153 - check("void foo() {\n" - " double dest = 23.0;\n" - " char* const source = (char*) malloc(sizeof(dest));\n" - " memcpy(&dest, source + sizeof(double), sizeof(dest));\n" - "}", false, "test.cpp", false); - ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds.\n", errout.str()); + checkstd("void foo() {\n" + " double dest = 23.0;\n" + " char* const source = (char*) malloc(sizeof(dest));\n" + " memcpy(&dest, source + sizeof(double), sizeof(dest));\n" + "}"); + TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds.\n", "", errout.str()); - check("void foo() {\n" - " double dest = 23.0;\n" - " char* const source = (char*) malloc(2 * sizeof(dest));\n" - " memcpy(&dest, source + sizeof(double), sizeof(dest));\n" - "}", false, "test.cpp", false); + checkstd("void foo() {\n" + " double dest = 23.0;\n" + " char* const source = (char*) malloc(2 * sizeof(dest));\n" + " memcpy(&dest, source + sizeof(double), sizeof(dest));\n" + "}"); ASSERT_EQUALS("", errout.str()); } @@ -3085,148 +3104,137 @@ private: } void strncat1() { - check("void f(char *a, char *b)\n" - "{\n" - " char str[16];\n" - " strncpy(str, a, 10);\n" - " strncat(str, b, 10);\n" - "}"); - ASSERT_EQUALS("[test.cpp:5]: (warning) Dangerous usage of strncat - 3rd parameter is the maximum number of characters to append.\n", errout.str()); - } - - void strncat2() { - check("void f(char *a)\n" - "{\n" - " char str[5];\n" - " strncat(str, a, 5);\n" - "}"); + checkstd("void f(char *a, char *b) {\n" + " char str[16];\n" + " strncpy(str, a, 10);\n" + " strncat(str, b, 10);\n" + "}"); ASSERT_EQUALS("[test.cpp:4]: (warning) Dangerous usage of strncat - 3rd parameter is the maximum number of characters to append.\n", errout.str()); } + void strncat2() { + checkstd("void f(char *a) {\n" + " char str[5];\n" + " strncat(str, a, 5);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Dangerous usage of strncat - 3rd parameter is the maximum number of characters to append.\n", errout.str()); + } + void strncat3() { - check("struct Foo { char a[4]; };\n" - "void f(char *a)\n" - "{\n" - " struct Foo x;\n" - " strncat(x.a, a, 5);\n" - "}"); - ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: x.a\n", errout.str()); + checkstd("struct Foo { char a[4]; };\n" + "void f(char *a) {\n" + " struct Foo x;\n" + " strncat(x.a, a, 5);\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: x.a\n", errout.str()); } void strncat4() { - check("void f(char *a) {\n" - " char str[5];\n" - " strncat(str, \"foobar\", 5);\n" - "}"); + checkstd("void f(char *a) {\n" + " char str[5];\n" + " strncat(str, \"foobar\", 5);\n" + "}"); ASSERT_EQUALS("[test.cpp:3]: (warning) Dangerous usage of strncat - 3rd parameter is the maximum number of characters to append.\n", errout.str()); } void strcat1() { - check("struct Foo { char a[4]; };\n" - "void f()\n" - "{\n" - " struct Foo x;\n" - " strcat(x.a, \"aa\");\n" - " strcat(x.a, \"aa\");\n" - "}"); - ASSERT_EQUALS("[test.cpp:6]: (error) Buffer is accessed out of bounds.\n", errout.str()); + checkstd("struct Foo { char a[4]; };\n" + "void f() {\n" + " struct Foo x;\n" + " strcat(x.a, \"aa\");\n" + " strcat(x.a, \"aa\");\n" + "}"); + ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds.\n", errout.str()); } void strcat2() { - check("struct Foo { char a[5]; };\n" - "void f()\n" - "{\n" - " struct Foo x;\n" - " strcat(x.a, \"aa\");\n" - " strcat(x.a, \"aa\");\n" - "}"); + checkstd("struct Foo { char a[5]; };\n" + "void f() {\n" + " struct Foo x;\n" + " strcat(x.a, \"aa\");\n" + " strcat(x.a, \"aa\");\n" + "}"); ASSERT_EQUALS("", errout.str()); } void strcat3() { - check("void f() {\n" - " INT str[10];\n" - " strcat(str, \"aa\");\n" - "}"); + checkstd("void f() {\n" + " INT str[10];\n" + " strcat(str, \"aa\");\n" + "}"); ASSERT_EQUALS("", errout.str()); } // memchr/memset/memcpy/etc void memfunc1() { - check("struct S {\n" - " char a[5];\n" - "};\n" - "void f()\n" - "{\n" - " S s;\n" - " memset(s.a, 0, 10);\n" - "}"); - ASSERT_EQUALS("[test.cpp:7]: (error) Buffer is accessed out of bounds: s.a\n", errout.str()); + checkstd("struct S {\n" + " char a[5];\n" + "};\n" + "void f() {\n" + " S s;\n" + " memset(s.a, 0, 10);\n" + "}"); + ASSERT_EQUALS("[test.cpp:6]: (error) Buffer is accessed out of bounds: s.a\n", errout.str()); - check("void f()\n" - "{\n" - " char str[5];\n" - " memset(str, 0, 10);\n" - "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str\n", errout.str()); + checkstd("void f() {\n" + " char str[5];\n" + " memset(str, 0, 10);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: str\n", errout.str()); - check("void f()\n" - "{\n" - " char a[5], b[50];\n" - " memcpy(a, b, 10);\n" - "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: a\n", errout.str()); + checkstd("void f() {\n" + " char a[5], b[50];\n" + " memcpy(a, b, 10);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: a\n", errout.str()); - check("void f()\n" - "{\n" - " char a[5], b[50];\n" - " memmove(a, b, 10);\n" - "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: a\n", errout.str()); + checkstd("void f() {\n" + " char a[5], b[50];\n" + " memmove(a, b, 10);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: a\n", errout.str()); // Ticket #909 - check("void f()\n" - "{\n" - " char * pch;\n" - " char str[] = \"Example string\";\n" - " pch = memchr (str, 'p', 16);\n" - "}"); - ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: str\n", errout.str()); + checkstd("void f() {\n" + " char * pch;\n" + " char str[] = \"Example string\";\n" + " pch = memchr (str, 'p', 16);\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str\n", errout.str()); } // ticket #2121 - buffer access out of bounds when using uint32_t void memfunc2() { - check("void f()\n" - "{\n" - " unknown_type_t buf[4];\n" - " memset(buf, 0, 100);\n" - "}"); + checkstd("void f() {\n" + " unknown_type_t buf[4];\n" + " memset(buf, 0, 100);\n" + "}"); ASSERT_EQUALS("", errout.str()); } // ticket #1659 - overflowing variable when using memcpy void memfunc3() { - check("void f() { \n" - "char str1[]=\"Sample string\";\n" - "char str2;\n" - "memcpy (&str2,str1,13);\n" // <-- strlen(str1)+1 = 13 - "}"); + checkstd("void f() { \n" + " char str1[]=\"Sample string\";\n" + " char str2;\n" + " memcpy (&str2,str1,13);\n" // <-- strlen(str1)+1 = 13 + "}"); TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str1\n","", errout.str()); - check("void f() {\n" - " char a[10];\n" - " char str1[] = \"abcdef\";\n" - " memset(a, 0, 11);\n" // <-- strlen(str1) + 5 = 11 - "}"); + checkstd("void f() {\n" + " char a[10];\n" + " char str1[] = \"abcdef\";\n" + " memset(a, 0, 11);\n" // <-- strlen(str1) + 5 = 11 + "}"); ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: a\n", errout.str()); - check("void f() { \n" - "char str1[]=\"Sample string\";\n" - "char str2;\n" - "memcpy (&str2,str1,15);\n" // <-- strlen(str1) + 1 = 15 - "}"); + checkstd("void f() { \n" + "char str1[]=\"Sample string\";\n" + "char str2;\n" + "memcpy (&str2,str1,15);\n" // <-- strlen(str1) + 1 = 15 + "}"); ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str1\n", errout.str()); } @@ -3245,15 +3253,15 @@ private: void varid2() { - check("void foo()\n" - "{\n" - " char str[10];\n" - " if (str[0])\n" - " {\n" - " char str[50];\n" - " memset(str,0,50);\n" - " }\n" - "}"); + checkstd("void foo()\n" + "{\n" + " char str[10];\n" + " if (str[0])\n" + " {\n" + " char str[50];\n" + " memset(str,0,50);\n" + " }\n" + "}"); ASSERT_EQUALS("", errout.str()); } @@ -3401,26 +3409,26 @@ private: } void malloc_memset() { - check("void f() {\n" - " char *p = malloc(10);\n" - " memset(p,0,100);\n" - "}"); + checkstd("void f() {\n" + " char *p = malloc(10);\n" + " memset(p,0,100);\n" + "}"); ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", errout.str()); } void memset1() { - check("void foo()\n" - "{\n" - " char s[10];\n" - " memset(s, 5, '*');\n" - "}"); + checkstd("void foo()\n" + "{\n" + " char s[10];\n" + " memset(s, 5, '*');\n" + "}"); ASSERT_EQUALS("[test.cpp:4]: (warning) The size argument is given as a char constant.\n", errout.str()); - check("void foo()\n" - "{\n" - " int* x[5];\n" - " memset(x, 0, sizeof(x));\n" - "}",false,"test.cpp",false); + checkstd("void foo()\n" + "{\n" + " int* x[5];\n" + " memset(x, 0, sizeof(x));\n" + "}"); ASSERT_EQUALS("", errout.str()); } @@ -3514,35 +3522,34 @@ private: } void strncpy1() { - check("void f() {\n" - " char c[7];\n" - " strncpy(c, \"hello\", 7);\n" - "}"); + checkstd("void f() {\n" + " char c[7];\n" + " strncpy(c, \"hello\", 7);\n" + "}"); ASSERT_EQUALS("", errout.str()); - check("void f() {\n" - " char c[6];\n" - " strncpy(c,\"hello\",6);\n" - "}"); + checkstd("void f() {\n" + " char c[6];\n" + " strncpy(c,\"hello\",6);\n" + "}"); ASSERT_EQUALS("", errout.str()); - check("void f() {\n" - " char c[5];\n" - " strncpy(c,\"hello\",6);\n" - "}"); + checkstd("void f() {\n" + " char c[5];\n" + " strncpy(c,\"hello\",6);\n" + "}"); ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: c\n", errout.str()); - check("void f() {\n" - " char c[6];\n" - " strncpy(c,\"hello!\",7);\n" - "}"); + checkstd("void f() {\n" + " char c[6];\n" + " strncpy(c,\"hello!\",7);\n" + "}"); ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: c\n", errout.str()); - check("struct AB { char a[10]; };\n" - "void foo(AB *ab)\n" - "{\n" - " strncpy(x, ab->a, 100);\n" - "}"); + checkstd("struct AB { char a[10]; };\n" + "void foo(AB *ab) {\n" + " strncpy(x, ab->a, 100);\n" + "}"); ASSERT_EQUALS("", errout.str()); } diff --git a/test/testlibrary.cpp b/test/testlibrary.cpp index 4bb96b65a..2556bc896 100644 --- a/test/testlibrary.cpp +++ b/test/testlibrary.cpp @@ -34,6 +34,7 @@ private: TEST_CASE(function_arg); TEST_CASE(function_arg_any); TEST_CASE(function_arg_valid); + TEST_CASE(function_arg_minsize); TEST_CASE(memory); TEST_CASE(memory2); // define extra "free" allocation functions TEST_CASE(resource); @@ -158,6 +159,41 @@ private: ASSERT_EQUALS(false, library.isargvalid("foo", 5, 2)); } + void function_arg_minsize() const { + const char xmldata[] = "\n" + "\n" + " \n" + " \n" + " \n" + " \n" + ""; + tinyxml2::XMLDocument doc; + doc.Parse(xmldata, sizeof(xmldata)); + + Library library; + library.load(doc); + + // arg1: type=strlen arg2 + const std::list *minsizes = library.argminsizes("foo",1); + 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::Type::STRLEN, m.type); + ASSERT_EQUALS(2, m.arg); + } + + // arg2: type=argvalue arg3 + minsizes = library.argminsizes("foo", 2); + 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::Type::ARGVALUE, m.type); + ASSERT_EQUALS(3, m.arg); + } + } + void memory() const { const char xmldata[] = "\n" "\n"