walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

insecureCmdLineArgs: Fixed FN in case strdup() copies argv[]. (#1438) 

* insecureCmdLineArgs: Fixed FN in case strdup() copies argv[].

* Formatted the code. There are no functional changes intended.

* Changes due to review comments from Daniel.
This commit is contained in:
orbitcowboy 2018-10-19 11:04:15 +02:00 committed by GitHub
parent f228897641
commit 0858488825
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 34 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
lib/checkbufferoverrun.cpp
View File

@ -1833,6 +1833,10 @@ void CheckBufferOverrun::checkInsecureCmdLineArgs()
if (tok->varId() == argvVarid) if (tok->varId() == argvVarid)
break; break;
// Update varid in case the input is copied by strdup()
if (Token::Match(tok->tokAt(-2), "%var% = strdup ( %varid%", argvVarid))
argvVarid = tok->tokAt(-2)->varId();
// Match common patterns that can result in a buffer overrun // Match common patterns that can result in a buffer overrun
// e.g. strcpy(buffer, argv[0]) // e.g. strcpy(buffer, argv[0])
if (Token::Match(tok, "strcpy|strcat (")) { if (Token::Match(tok, "strcpy|strcat (")) {
@ -1841,7 +1845,9 @@ void CheckBufferOverrun::checkInsecureCmdLineArgs()
tok = nextArgument; tok = nextArgument;
else else
continue; // Ticket #7964 continue; // Ticket #7964
if (Token::Match(tok, "* %varid%", argvVarid) || Token::Match(tok, "%varid% [", argvVarid)) if (Token::Match(tok, "* %varid%", argvVarid) || // e.g. strcpy(buf, * ptr)
Token::Match(tok, "%varid% [", argvVarid) || // e.g. strcpy(buf, argv[1])
Token::Match(tok, "%varid%", argvVarid)) // e.g. strcpy(buf, pointer)
cmdLineArgsError(tok); cmdLineArgsError(tok);
} }
} }

29
test/testbufferoverrun.cpp
View File

@ -235,7 +235,7 @@ private:
TEST_CASE(executionPaths5); // Ticket #2920 - False positive when size is unknown TEST_CASE(executionPaths5); // Ticket #2920 - False positive when size is unknown
TEST_CASE(executionPaths6); // unknown types TEST_CASE(executionPaths6); // unknown types
TEST_CASE(cmdLineArgs1); TEST_CASE(insecureCmdLineArgs);
TEST_CASE(checkBufferAllocatedWithStrlen); TEST_CASE(checkBufferAllocatedWithStrlen);
TEST_CASE(scope); // handling different scopes TEST_CASE(scope); // handling different scopes
@ -3750,7 +3750,32 @@ private:
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[10]' accessed at index 1000, which is out of bounds.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[10]' accessed at index 1000, which is out of bounds.\n", errout.str());
} }
void cmdLineArgs1() { void insecureCmdLineArgs() {
check("int main(int argc, char *argv[])\n"
"{\n"
" if(argc>1)\n"
" {\n"
" char buf[2];\n"
" char *p = strdup(argv[1]);\n"
" strcpy(buf,p);\n"
" free(p);\n"
" }\n"
" return 0;\n"
"}");
ASSERT_EQUALS("[test.cpp:7]: (error) Buffer overrun possible for long command line arguments.\n", errout.str());
check("int main(int argc, char *argv[])\n"
"{\n"
" if(argc>1)\n"
" {\n"
" char buf[2] = {'\\0','\\0'};\n"
" char *p = strdup(argv[1]);\n"
" strcat(buf,p);\n"
" free(p);\n"
" }\n"
" return 0;\n"
"}");
ASSERT_EQUALS("[test.cpp:7]: (error) Buffer overrun possible for long command line arguments.\n", errout.str());
check("int main(const int argc, char* argv[])\n" check("int main(const int argc, char* argv[])\n"
"{\n" "{\n"