From a6e8270474ae27f4127f7e8c02faba6961b618d9 Mon Sep 17 00:00:00 2001
From: orbitcowboy <ettl.martin78@googlemail.com>
Date: Mon, 15 Oct 2018 10:05:43 +0200
Subject: [PATCH]  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.
---
 lib/checkbufferoverrun.cpp | 26 +++++++++++++++++++-------
 test/testbufferoverrun.cpp | 38 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp
index ccd2fb3cf..e4aa187b5 100644
--- a/lib/checkbufferoverrun.cpp
+++ b/lib/checkbufferoverrun.cpp
@@ -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);
                 }
             }
diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
index 0a0ddaafd..5afdd23e8 100644
--- a/test/testbufferoverrun.cpp
+++ b/test/testbufferoverrun.cpp
@@ -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"