diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index e4aa187b5..1ee4089ce 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -1833,6 +1833,10 @@ void CheckBufferOverrun::checkInsecureCmdLineArgs() if (tok->varId() == argvVarid) 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 // e.g. strcpy(buffer, argv[0]) if (Token::Match(tok, "strcpy|strcat (")) { @@ -1841,7 +1845,9 @@ void CheckBufferOverrun::checkInsecureCmdLineArgs() tok = nextArgument; else 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); } } diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 5afdd23e8..eb1e0a381 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -235,7 +235,7 @@ private: TEST_CASE(executionPaths5); // Ticket #2920 - False positive when size is unknown TEST_CASE(executionPaths6); // unknown types - TEST_CASE(cmdLineArgs1); + TEST_CASE(insecureCmdLineArgs); TEST_CASE(checkBufferAllocatedWithStrlen); 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()); } - 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" "{\n"