insecureCmdLineArgs: Fixed false negatives in case arguments are const. (#1419)

* insecureCmdLineArgs: Fixed false negatives in case arguments are const.

* Formatted the code, there are functional changes.

* Simplified matching as suggested by Daniel.
This commit is contained in:
orbitcowboy 2018-10-15 10:05:43 +02:00 committed by GitHub
parent 613dc19b68
commit a6e8270474
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 56 additions and 8 deletions

View File

@ -1803,12 +1803,24 @@ void CheckBufferOverrun::checkInsecureCmdLineArgs()
const Token* tok = function->token;
// Get the name of the argv variable
unsigned int varid = 0;
if (Token::Match(tok, "main ( int %var% , char * %var% [ ] ,|)")) {
varid = tok->tokAt(7)->varId();
unsigned int argvVarid = 0;
if (Token::simpleMatch(tok, "main ("))
tok = tok->tokAt(2);
else
continue;
} else if (Token::Match(tok, "main ( int %var% , char * * %var% ,|)")) {
varid = tok->tokAt(8)->varId();
if (Token::Match(tok, "const| int %var% ,"))
tok = tok->nextArgument();
else
continue;
if (Token::Match(tok, "char * %var% [ ] ,|)")) {
argvVarid = tok->tokAt(2)->varId();
} else if (Token::Match(tok, "char * * %var% ,|)") ||
Token::Match(tok, "const char * %var% [ ] ,|)")) {
argvVarid = tok->tokAt(3)->varId();
} else if (Token::Match(tok, "const char * * %var% ,|)")) {
argvVarid = tok->tokAt(4)->varId();
} else
continue;
@ -1818,7 +1830,7 @@ void CheckBufferOverrun::checkInsecureCmdLineArgs()
// Search within main() for possible buffer overruns involving argv
for (const Token* end = tok->link(); tok != end; tok = tok->next()) {
// If argv is modified or tested, its size may be being limited properly
if (tok->varId() == varid)
if (tok->varId() == argvVarid)
break;
// Match common patterns that can result in a buffer overrun
@ -1829,7 +1841,7 @@ void CheckBufferOverrun::checkInsecureCmdLineArgs()
tok = nextArgument;
else
continue; // Ticket #7964
if (Token::Match(tok, "* %varid%", varid) || Token::Match(tok, "%varid% [", varid))
if (Token::Match(tok, "* %varid%", argvVarid) || Token::Match(tok, "%varid% [", argvVarid))
cmdLineArgsError(tok);
}
}

View File

@ -3751,7 +3751,22 @@ private:
}
void cmdLineArgs1() {
check("int main(int argc, char* argv[])\n"
check("int main(const 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, const 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(const int argc, const char* argv[])\n"
"{\n"
" char prog[10];\n"
" strcpy(prog, argv[0]);\n"
@ -3779,6 +3794,27 @@ private:
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long command line arguments.\n", errout.str());
check("int main(const int argc, const char **argv, char **envp)\n"
"{\n"
" 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, const char **argv, char **envp)\n"
"{\n"
" 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(const int argc, char **argv, char **envp)\n"
"{\n"
" 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 **options)\n"
"{\n"
" char prog[10];\n"