CheckBufferOverrun: Remove hardcoding for sprintf and rely on cfg configuration instead

This commit is contained in:
Daniel Marjamäki 2015-02-12 17:29:36 +01:00
parent baba720e58
commit 9aad4fa8ca
4 changed files with 127 additions and 249 deletions

View File

@ -5238,6 +5238,9 @@
<noreturn>false</noreturn>
<leak-ignore/>
<formatstr/>
<arg nr="1">
<minsize type="strlen" arg="2"/>
</arg>
<arg nr="2">
<not-null/>
<formatstr/>

View File

@ -281,7 +281,7 @@ static bool bailoutIfSwitch(const Token *tok, const unsigned int varid)
}
//---------------------------------------------------------------------------
static bool checkMinSizes(const std::list<Library::ArgumentChecks::MinSize> &minsizes, const Token * const ftok, const std::size_t arraySize, const Token **charSizeToken)
static bool checkMinSizes(const std::list<Library::ArgumentChecks::MinSize> &minsizes, const Token * const ftok, const std::size_t arraySize, const Token **charSizeToken, const Settings * const settings)
{
if (charSizeToken)
*charSizeToken = nullptr;
@ -318,9 +318,24 @@ static bool checkMinSizes(const std::list<Library::ArgumentChecks::MinSize> &min
}
break;
case Library::ArgumentChecks::MinSize::STRLEN: {
const Token *strtoken = argtok->getValueTokenMaxStrLength();
if (strtoken && Token::getStrLength(strtoken) >= arraySize)
error = true;
if (settings->library.isargformatstr(ftok,minsize->arg)) {
std::list<const Token *> parameters;
const std::string &formatstr(argtok->str());
const Token *argtok2 = argtok;
while ((argtok2 = argtok2->nextArgument()) != nullptr) {
if (Token::Match(argtok2, "%num%|%str% [,)]"))
parameters.push_back(argtok2);
else
parameters.push_back(nullptr);
}
const MathLib::bigint len = CheckBufferOverrun::countSprintfLength(formatstr, parameters);
if (len > arraySize + 2U)
error = true;
} else {
const Token *strtoken = argtok->getValueTokenMaxStrLength();
if (strtoken && Token::getStrLength(strtoken) >= arraySize)
error = true;
}
}
break;
case Library::ArgumentChecks::MinSize::SIZEOF:
@ -347,7 +362,7 @@ void CheckBufferOverrun::checkFunctionParameter(const Token &ftok, unsigned int
arraySize *= arrayInfo.num(i);
const Token *charSizeToken = nullptr;
if (checkMinSizes(*minsizes, &ftok, (std::size_t)arraySize, &charSizeToken))
if (checkMinSizes(*minsizes, &ftok, (std::size_t)arraySize, &charSizeToken, _settings))
bufferOverrunError(callstack, arrayInfo.varname());
if (charSizeToken)
sizeArgumentAsCharError(charSizeToken);
@ -677,12 +692,6 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str
}
}
// sprintf..
const std::string sprintfPattern = declarationId > 0 ? std::string("sprintf ( %varid% , %str% [,)]") : ("sprintf ( " + varnames + " , %str% [,)]");
if (Token::Match(tok, sprintfPattern.c_str(), declarationId)) {
checkSprintfCall(tok, static_cast<unsigned int>(total_size));
}
// snprintf..
const std::string snprintfPattern = declarationId > 0 ? std::string("snprintf ( %varid% , %num% ,") : ("snprintf ( " + varnames + " , %num% ,");
if (Token::Match(tok, snprintfPattern.c_str(), declarationId)) {
@ -965,11 +974,6 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
}
}
if (Token::Match(tok, "sprintf ( %varid% , %str% [,)]", declarationId)) {
checkSprintfCall(tok, total_size);
}
// snprintf..
if (total_size > 0 && Token::Match(tok, "snprintf ( %varid% , %num% ,", declarationId)) {
const MathLib::bigint n = MathLib::toLongNumber(tok->strAt(4));
@ -1471,35 +1475,6 @@ MathLib::bigint CheckBufferOverrun::countSprintfLength(const std::string &input_
return (MathLib::bigint)input_string_size;
}
void CheckBufferOverrun::checkSprintfCall(const Token *tok, const MathLib::bigint size)
{
if (size == 0)
return;
std::list<const Token*> parameters;
const Token* vaArg = tok->tokAt(2)->nextArgument()->nextArgument();
while (vaArg) {
if (Token::Match(vaArg->next(), "[,)]")) {
if (vaArg->type() == Token::eString)
parameters.push_back(vaArg);
else if (vaArg->isNumber())
parameters.push_back(vaArg);
else
parameters.push_back(nullptr);
} else // Parameter is more complex than just a value or variable. Ignore it for now and skip to next token.
parameters.push_back(nullptr);
vaArg = vaArg->nextArgument();
}
MathLib::bigint len = countSprintfLength(tok->tokAt(2)->nextArgument()->strValue(), parameters);
if (len > size) {
bufferOverrunError(tok);
}
}
//---------------------------------------------------------------------------
@ -1547,10 +1522,6 @@ void CheckBufferOverrun::checkBufferAllocatedWithStrlen()
if (Token::Match(tok, "strcpy ( %varid% , %var% )", dstVarId) &&
tok->tokAt(4)->varId() == srcVarId) {
bufferOverrunError(tok);
} else if (Token::Match(tok, "sprintf ( %varid% , %str% , %var% )", dstVarId) &&
tok->tokAt(6)->varId() == srcVarId &&
tok->strAt(4).find("%s") != std::string::npos) {
bufferOverrunError(tok);
}
}
if (!tok)
@ -1582,7 +1553,7 @@ void CheckBufferOverrun::checkStringArgument()
const std::list<Library::ArgumentChecks::MinSize> *minsizes = _settings->library.argminsizes(tok, argnr);
if (!minsizes)
continue;
if (checkMinSizes(*minsizes, tok, Token::getStrSize(strtoken), nullptr))
if (checkMinSizes(*minsizes, tok, Token::getStrSize(strtoken), nullptr, _settings))
bufferOverrunError(argtok);
}
}
@ -1635,14 +1606,6 @@ void CheckBufferOverrun::checkInsecureCmdLineArgs()
Token::Match(tok, "strcpy|strcat ( %name% , %varid% [", varid)) {
cmdLineArgsError(tok);
tok = tok->linkAt(1);
} else if (Token::Match(tok, "sprintf ( %name% , %str% , %varid% [", varid) &&
tok->strAt(4).find("%s") != std::string::npos) {
cmdLineArgsError(tok);
tok = tok->linkAt(1);
} else if (Token::Match(tok, "sprintf ( %name% , %str% , * %varid%", varid) &&
tok->strAt(4).find("%s") != std::string::npos) {
cmdLineArgsError(tok);
tok = tok->linkAt(1);
}
}
}

View File

@ -82,14 +82,6 @@ public:
*/
static MathLib::bigint countSprintfLength(const std::string &input_string, const std::list<const Token*> &parameters);
/**
* @brief %Check code that matches: "sprintf ( %varid% , %str% [,)]" when varid is not 0,
* and report found errors.
* @param tok The "sprintf" token.
* @param size The size of the buffer where sprintf is writing.
*/
void checkSprintfCall(const Token *tok, const MathLib::bigint size);
/** Check for buffer overruns - locate struct variables and check them with the .._CheckScope function */
void checkStructVariable();

View File

@ -183,7 +183,6 @@ private:
TEST_CASE(buffer_overrun_14);
TEST_CASE(buffer_overrun_15); // ticket #1787
TEST_CASE(buffer_overrun_16);
TEST_CASE(buffer_overrun_17); // ticket #2548
TEST_CASE(buffer_overrun_18); // ticket #2576 - for, calculation with loop variable
TEST_CASE(buffer_overrun_19); // #2597 - class member with unknown type
TEST_CASE(buffer_overrun_20); // #2986 (segmentation fault)
@ -210,17 +209,6 @@ private:
TEST_CASE(pointer_out_of_bounds_2);
TEST_CASE(pointer_out_of_bounds_sub);
TEST_CASE(sprintf1);
TEST_CASE(sprintf2);
TEST_CASE(sprintf3);
TEST_CASE(sprintf4);
TEST_CASE(sprintf5);
TEST_CASE(sprintf6);
TEST_CASE(sprintf7);
TEST_CASE(sprintf8);
TEST_CASE(sprintf9);
TEST_CASE(sprintf10);
TEST_CASE(snprintf1);
TEST_CASE(snprintf2);
TEST_CASE(snprintf4);
@ -258,6 +246,7 @@ private:
TEST_CASE(counter_test);
TEST_CASE(minsize_argvalue);
TEST_CASE(minsize_sizeof);
TEST_CASE(minsize_strlen);
TEST_CASE(minsize_mul);
TEST_CASE(unknownType);
@ -2343,7 +2332,7 @@ private:
// ticket #900
check("void f() {\n"
" char *a = new char(30);\n"
" sprintf(a, \"%s\", \"b\");\n"
" strcpy(a, \"b\");\n"
" delete a;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", errout.str());
@ -2419,7 +2408,7 @@ private:
checkstd("void f(char *a) {\n"
" char *b = malloc(strlen(a));\n"
" sprintf(b, \"%s\", a);\n"
" strcpy(b, a);\n"
" return b;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", errout.str());
@ -2427,14 +2416,14 @@ private:
void buffer_overrun_15() { // ticket #1787
check("class A : public B {\n"
" char val[12];\n"
" char val[2];\n"
" void f(int i, int ii);\n"
"};\n"
"void A::f(int i, int ii)\n"
"{\n"
" sprintf(val, \"drive_%d_partition_%d_size\", i, ii) ;\n"
" strcpy(val, \"ab\") ;\n"
"}");
ASSERT_EQUALS("[test.cpp:7]: (error) Buffer is accessed out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:7]: (error) Buffer is accessed out of bounds: val\n", errout.str());
}
void buffer_overrun_16() {
@ -2459,14 +2448,6 @@ private:
ASSERT_EQUALS("", errout.str());
}
void buffer_overrun_17() { // ticket #2548
check("void f() {\n"
" char t[8];\n"
" sprintf(t, \"%s\", \"foo bar\");\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", errout.str());
}
void buffer_overrun_18() { // ticket #2576
check("class A {\n"
" void foo();\n"
@ -2912,135 +2893,6 @@ private:
ASSERT_EQUALS("[test.cpp:4]: (portability) Undefined behaviour, when 'i' is -20 the pointer arithmetic 'x-i' is out of bounds.\n", errout.str());
}
void sprintf1() {
check("void f()\n"
"{\n"
" char str[3];\n"
" sprintf(str, \"%s\", \"abc\");\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds.\n", errout.str());
check("void f()\n"
"{\n"
" char * c = new char[10];\n"
" sprintf(c, \"%s\", \"/usr/LongLongLongLongUserName/bin/LongLongApplicationName\");\n"
" delete [] c;\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds.\n", errout.str());
}
void sprintf2() {
check("int getnumber();\n"
"void f()\n"
"{\n"
" char str[5];\n"
" sprintf(str, \"%d: %s\", getnumber(), \"abcde\");\n"
"}");
ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds.\n", errout.str());
}
void sprintf3() {
check("void f()\n"
"{\n"
" char str[3];\n"
" sprintf(str, \"test\");\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds.\n", errout.str());
check("void f()\n"
"{\n"
" char str[5];\n"
" sprintf(str, \"test%s\", \"\");\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void sprintf4() {
// ticket #690
check("void f()\n"
"{\n"
" char a[3];\n"
" sprintf(a, \"%02ld\", 99);\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void sprintf5() {
// ticket #729
check("void f(int condition)\n"
"{\n"
" char buf[3];\n"
" sprintf(buf, \"%s\", condition ? \"11\" : \"22\");\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void sprintf6() {
check("void f(int condition)\n"
"{\n"
" char buf[3];\n"
" sprintf(buf, \"%s\", condition ? \"11\" : \"222\");\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds.\n", "", errout.str());
}
void sprintf7() {
check("struct Foo { char a[1]; };\n"
"void f()\n"
"{\n"
" struct Foo x;\n"
" sprintf(x.a, \"aa\");\n"
"}");
ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds.\n", errout.str());
// This is out of bounds if 'sizeof(ABC)' is 1 (No padding)
check("struct Foo { char a[1]; };\n"
"void f()\n"
"{\n"
" struct Foo *x = malloc(sizeof(Foo));\n"
" sprintf(x.a, \"aa\");\n"
" free(x);\n"
"}");
TODO_ASSERT_EQUALS("error", "", errout.str());
check("struct Foo { char a[1]; };\n"
"void f()\n"
"{\n"
" struct Foo *x = malloc(sizeof(Foo) + 10);\n"
" sprintf(x.a, \"aa\");\n"
" free(x);\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void sprintf8() {
check("struct Foo { char a[3]; };\n"
"void f()\n"
"{\n"
" struct Foo x;\n"
" sprintf(x.a, \"aa\");\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void sprintf9() {
check("void f()\n"
"{\n"
" gchar str[3];\n"
" sprintf(str, \"1\");\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void sprintf10() {
check("void f()\n"
"{\n"
" TString str = \"\";\n"
" sprintf(str, \"1\");\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void snprintf1() {
check("void f()\n"
"{\n"
@ -3501,9 +3353,11 @@ private:
ASSERT_EQUALS(4, CheckBufferOverrun::countSprintfLength("%%%%%d", unknownParameter));
Token strTok(0);
strTok.str("\"12345\"");
std::list<const Token*> stringAsParameter;
stringAsParameter.push_back(&strTok);
strTok.str("\"\"");
ASSERT_EQUALS(4, CheckBufferOverrun::countSprintfLength("str%s", stringAsParameter));
strTok.str("\"12345\"");
ASSERT_EQUALS(9, CheckBufferOverrun::countSprintfLength("str%s", stringAsParameter));
ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%-4s", stringAsParameter));
ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%-5s", stringAsParameter));
@ -3552,7 +3406,6 @@ private:
multipleParams.push_back(&numTok);
ASSERT_EQUALS(15, CheckBufferOverrun::countSprintfLength("str%s%d%d", multipleParams));
ASSERT_EQUALS(26, CheckBufferOverrun::countSprintfLength("str%-6s%08ld%08ld", multipleParams));
}
void minsize_argvalue() {
@ -3630,6 +3483,7 @@ private:
doc.Parse(xmldata, sizeof(xmldata));
settings.library.load(doc);
// strncpy...
check("void f() {\n"
" char c[7];\n"
" strncpy(c, \"hello\", 7);\n"
@ -3660,14 +3514,99 @@ private:
"}", settings);
ASSERT_EQUALS("", errout.str());
// #3168
check("void a(char *p) { strncpy(p,\"hello world!\",10); }\n"
check("void a(char *p) { strncpy(p,\"hello world!\",10); }\n" // #3168
"void b() {\n"
" char buf[5];\n"
" a(buf);"
"}", settings);
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1]: (error) Buffer is accessed out of bounds: buf\n", errout.str());
}
void minsize_strlen() {
Settings settings;
const char xmldata[] = "<?xml version=\"1.0\"?>\n"
"<def>\n"
" <function name=\"mysprintf\">\n"
" <noreturn>false</noreturn>\n"
" <formatstr/>\n"
" <arg nr=\"1\">\n"
" <minsize type=\"strlen\" arg=\"2\"/>\n"
" </arg>\n"
" <arg nr=\"2\">\n"
" <formatstr/>\n"
" </arg>\n"
" </function>\n"
"</def>";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
settings.library.load(doc);
// formatstr..
check("void f() {\n"
" char str[3];\n"
" mysprintf(str, \"test\");\n"
"}", settings);
ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: str\n", errout.str());
check("void f() {\n"
" char str[5];\n"
" mysprintf(str, \"%s\", \"abcde\");\n"
"}", settings);
ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: str\n", errout.str());
check("int getnumber();\n"
"void f()\n"
"{\n"
" char str[5];\n"
" mysprintf(str, \"%d: %s\", getnumber(), \"abcde\");\n"
"}", settings);
ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: str\n", errout.str());
check("void f() {\n"
" char str[5];\n"
" mysprintf(str, \"test%s\", \"\");\n"
"}", settings);
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" char *str = new char[5];\n"
" mysprintf(str, \"abcde\");\n"
"}", settings);
ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", errout.str());
check("void f(int condition) {\n"
" char str[5];\n"
" mysprintf(str, \"test%s\", condition ? \"12\" : \"34\");\n"
"}", settings);
ASSERT_EQUALS("", errout.str());
check("void f(int condition) {\n"
" char str[5];\n"
" mysprintf(str, \"test%s\", condition ? \"12\" : \"345\");\n"
"}", settings);
TODO_ASSERT_EQUALS("error", "", errout.str());
check("struct Foo { char a[1]; };\n"
"void f() {\n"
" struct Foo x;\n"
" mysprintf(x.a, \"aa\");\n"
"}", settings);
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: x.a\n", errout.str());
// This is out of bounds if 'sizeof(ABC)' is 1 (No padding)
check("struct Foo { char a[1]; };\n"
"void f() {\n"
" struct Foo *x = malloc(sizeof(Foo));\n"
" mysprintf(x.a, \"aa\");\n"
"}", settings);
TODO_ASSERT_EQUALS("error", "", errout.str());
check("struct Foo { char a[1]; };\n"
"void f() {\n"
" struct Foo *x = malloc(sizeof(Foo) + 10);\n"
" mysprintf(x.a, \"aa\");\n"
"}", settings);
ASSERT_EQUALS("", errout.str());
}
void minsize_mul() {
Settings settings;
@ -3907,12 +3846,12 @@ private:
}
void cmdLineArgs1() {
check("int main(int argc, char* argv[])\n"
"{\n"
" char prog[10];\n"
" strcpy(prog, argv[0]);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long command line arguments.\n", errout.str());
check("int main(int argc, char* argv[])\n"
@ -3920,15 +3859,13 @@ private:
" char prog[10] = {'\\0'};\n"
" strcat(prog, argv[0]);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long command line arguments.\n", errout.str());
check("int main(int argc, char* argv[])\n"
"{\n"
" char prog[10];\n"
" sprintf(prog, \"%s\", argv[0]);\n"
" strcpy(prog, argv[0]);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long command line arguments.\n", errout.str());
check("int main(int argc, char **argv, char **envp)\n"
@ -3936,7 +3873,6 @@ private:
" char prog[10];\n"
" strcpy(prog, argv[0]);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long command line arguments.\n", errout.str());
check("int main(int argc, char **argv, char **envp)\n"
@ -3944,15 +3880,13 @@ private:
" char prog[10] = {'\\0'};\n"
" strcat(prog, argv[0]);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long command line arguments.\n", errout.str());
check("int main(int argc, char **argv, char **envp)\n"
"{\n"
" char prog[10];\n"
" sprintf(prog, \"%s\", argv[0]);\n"
" strcpy(prog, argv[0]);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long command line arguments.\n", errout.str());
check("int main(int argc, char **options)\n"
@ -3960,7 +3894,6 @@ private:
" char prog[10];\n"
" strcpy(prog, options[0]);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long command line arguments.\n", errout.str());
check("int main(int argc, char **options)\n"
@ -3968,15 +3901,13 @@ private:
" char prog[10] = {'\\0'};\n"
" strcat(prog, options[0]);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long command line arguments.\n", errout.str());
check("int main(int argc, char **options)\n"
"{\n"
" char prog[10];\n"
" sprintf(prog, \"%s\", *options);\n"
" strcpy(prog, *options);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long command line arguments.\n", errout.str());
check("int main(int argc, char **argv, char **envp)\n"
@ -3985,7 +3916,6 @@ private:
" if (strlen(argv[0]) < 10)\n"
" strcpy(prog, argv[0]);\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int main(int argc, char **argv, char **envp)\n"
@ -3994,15 +3924,6 @@ private:
" if (10 > strlen(argv[0]))\n"
" strcat(prog, argv[0]);\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int main(int argc, char **argv, char **envp)\n"
"{\n"
" char prog[10];\n"
" sprintf(prog, \"%p\", argv[0]);\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int main(int argc, char **argv, char **envp)\n"
@ -4011,14 +3932,13 @@ private:
" argv[0][0] = '\\0';\n"
" strcpy(prog, argv[0]);\n"
"}");
ASSERT_EQUALS("", errout.str());
// #5835
checkstd("int main(int argc, char* argv[]) {\n"
" char prog[10];\n"
" sprintf(prog, \"%s\", argv[0]);\n"
" sprintf(prog, \"%s\", argv[0]);\n"
" strcpy(prog, argv[0]);\n"
" strcpy(prog, argv[0]);\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Buffer overrun possible for long command line arguments.\n"
"[test.cpp:4]: (error) Buffer overrun possible for long command line arguments.\n", errout.str());