Fixed #568 (string functions with command line arguments may overflow buffer)
This commit is contained in:
Zachary Blair
parent
7082371849
commit
33b4254d33
|
|
@ -119,6 +119,11 @@ void CheckBufferOverrun::terminateStrncpyError(const Token *tok)
|
|||
reportError(tok, Severity::style, "terminateStrncpy", "After a strncpy() the buffer should be zero-terminated");
|
||||
}
|
||||
|
||||
void CheckBufferOverrun::cmdLineArgsError(const Token *tok)
|
||||
{
|
||||
reportError(tok, Severity::error, "insecureCmdLineArgs", "Buffer overrun possible for long cmd-line args");
|
||||
}
|
||||
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
|
|
@ -1229,6 +1234,7 @@ void CheckBufferOverrun::bufferOverrun()
|
|||
checkGlobalAndLocalVariable();
|
||||
checkStructVariable();
|
||||
checkBufferAllocatedWithStrlen();
|
||||
checkInsecureCmdLineArgs();
|
||||
}
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
|
|
@ -1489,6 +1495,85 @@ void CheckBufferOverrun::checkBufferAllocatedWithStrlen()
|
|||
|
||||
}
|
||||
}
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// Checking for buffer overflow caused by copying command line arguments
|
||||
// into fixed-sized buffers without checking to make sure that the command
|
||||
// line arguments will not overflow the buffer.
|
||||
//
|
||||
// int main(int argc, char* argv[])
|
||||
// {
|
||||
// char prog[10];
|
||||
// strcpy(prog, argv[0]); <-- Possible buffer overrun
|
||||
// }
|
||||
//---------------------------------------------------------------------------
|
||||
void CheckBufferOverrun::checkInsecureCmdLineArgs()
|
||||
{
|
||||
const char pattern[] = "main ( int %var% , char *";
|
||||
for (const Token *tok = Token::findmatch(_tokenizer->tokens(), pattern); tok; tok = Token::findmatch(tok->next(),pattern))
|
||||
{
|
||||
// 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();
|
||||
|
||||
}
|
||||
else if (Token::Match(tok, "main ( int %var% , char * * %var% ,|)"))
|
||||
{
|
||||
varid = tok->tokAt(8)->varId();
|
||||
}
|
||||
if (varid == 0)
|
||||
continue;
|
||||
|
||||
// Jump to the opening curly brace
|
||||
tok = tok->next()->link();
|
||||
if (!tok || !tok->next())
|
||||
continue;
|
||||
tok = tok->next();
|
||||
|
||||
// Search within main() for possible buffer overruns involving argv
|
||||
int indentlevel = -1;
|
||||
for (; tok && tok->next(); tok = tok->next())
|
||||
{
|
||||
if (tok->str() == "{")
|
||||
{
|
||||
++indentlevel;
|
||||
}
|
||||
|
||||
else if (tok->str() == "}")
|
||||
{
|
||||
--indentlevel;
|
||||
if (indentlevel < 0)
|
||||
return;
|
||||
}
|
||||
|
||||
// If argv is modified or tested, its size may be being limited properly
|
||||
if (tok->varId() == varid)
|
||||
break;
|
||||
|
||||
// Match common patterns that can result in a buffer overrun
|
||||
// e.g. strcpy(buffer, argv[0])
|
||||
if (Token::Match(tok, "strcpy|strcat ( %var% , * %varid%", varid) ||
|
||||
Token::Match(tok, "strcpy|strcat ( %var% , %varid% [", varid))
|
||||
{
|
||||
cmdLineArgsError(tok);
|
||||
}
|
||||
else if (Token::Match(tok, "sprintf ( %var% , %str% , %varid% [", varid) &&
|
||||
tok->tokAt(4)->str().find("%s") != std::string::npos)
|
||||
{
|
||||
cmdLineArgsError(tok);
|
||||
}
|
||||
else if (Token::Match(tok, "sprintf ( %var% , %str% , * %varid%", varid) &&
|
||||
tok->tokAt(4)->str().find("%s") != std::string::npos)
|
||||
{
|
||||
cmdLineArgsError(tok);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -97,6 +97,9 @@ public:
|
|||
|
||||
/** Check for buffer overruns due to allocating strlen(src) bytes instead of (strlen(src)+1) bytes before copying a string */
|
||||
void checkBufferAllocatedWithStrlen();
|
||||
|
||||
/** Check for buffer overruns due to copying command-line args to fixed-sized buffers without bounds checking */
|
||||
void checkInsecureCmdLineArgs();
|
||||
|
||||
/** Check for negative index */
|
||||
void negativeIndex();
|
||||
|
|
@ -180,6 +183,7 @@ public:
|
|||
void sizeArgumentAsChar(const Token *tok);
|
||||
void terminateStrncpyError(const Token *tok);
|
||||
void negativeIndexError(const Token *tok, long index);
|
||||
void cmdLineArgsError(const Token *tok);
|
||||
|
||||
void getErrorMessages()
|
||||
{
|
||||
|
|
@ -190,6 +194,7 @@ public:
|
|||
sizeArgumentAsChar(0);
|
||||
terminateStrncpyError(0);
|
||||
negativeIndexError(0, -1);
|
||||
cmdLineArgsError(0);
|
||||
}
|
||||
|
||||
std::string name() const
|
||||
|
|
|
|||
|
|
@ -170,6 +170,8 @@ private:
|
|||
TEST_CASE(crash); // Ticket #1587 - crash
|
||||
|
||||
TEST_CASE(executionPaths1);
|
||||
|
||||
TEST_CASE(cmdLineArgs1);
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -2291,6 +2293,116 @@ private:
|
|||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:7]: (error) Array 'buf[10][5]' index 1000 out of bounds\n", errout.str());
|
||||
}
|
||||
|
||||
void cmdLineArgs1()
|
||||
{
|
||||
check("int main(int argc, char* argv[])\n"
|
||||
"{\n"
|
||||
" char prog[10];\n"
|
||||
" strcpy(prog, argv[0]);\n"
|
||||
"}\n");
|
||||
|
||||
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long cmd-line args\n", errout.str());
|
||||
|
||||
check("int main(int argc, char* argv[])\n"
|
||||
"{\n"
|
||||
" char prog[10] = {'\\0'};\n"
|
||||
" strcat(prog, argv[0]);\n"
|
||||
"}\n");
|
||||
|
||||
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long cmd-line args\n", errout.str());
|
||||
|
||||
check("int main(int argc, char* argv[])\n"
|
||||
"{\n"
|
||||
" char prog[10];\n"
|
||||
" sprintf(prog, \"%s\", argv[0]);\n"
|
||||
"}\n");
|
||||
|
||||
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long cmd-line args\n", errout.str());
|
||||
|
||||
check("int main(int argc, char **argv, char **envp)\n"
|
||||
"{\n"
|
||||
" char prog[10];\n"
|
||||
" strcpy(prog, argv[0]);\n"
|
||||
"}\n");
|
||||
|
||||
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long cmd-line args\n", errout.str());
|
||||
|
||||
check("int main(int argc, char **argv, char **envp)\n"
|
||||
"{\n"
|
||||
" char prog[10] = {'\\0'};\n"
|
||||
" strcat(prog, argv[0]);\n"
|
||||
"}\n");
|
||||
|
||||
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long cmd-line args\n", errout.str());
|
||||
|
||||
check("int main(int argc, char **argv, char **envp)\n"
|
||||
"{\n"
|
||||
" char prog[10];\n"
|
||||
" sprintf(prog, \"%s\", argv[0]);\n"
|
||||
"}\n");
|
||||
|
||||
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long cmd-line args\n", errout.str());
|
||||
|
||||
check("int main(int argc, char **options)\n"
|
||||
"{\n"
|
||||
" char prog[10];\n"
|
||||
" strcpy(prog, options[0]);\n"
|
||||
"}\n");
|
||||
|
||||
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long cmd-line args\n", errout.str());
|
||||
|
||||
check("int main(int argc, char **options)\n"
|
||||
"{\n"
|
||||
" char prog[10] = {'\\0'};\n"
|
||||
" strcat(prog, options[0]);\n"
|
||||
"}\n");
|
||||
|
||||
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long cmd-line args\n", errout.str());
|
||||
|
||||
check("int main(int argc, char **options)\n"
|
||||
"{\n"
|
||||
" char prog[10];\n"
|
||||
" sprintf(prog, \"%s\", *options);\n"
|
||||
"}\n");
|
||||
|
||||
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer overrun possible for long cmd-line args\n", errout.str());
|
||||
|
||||
check("int main(int argc, char **argv, char **envp)\n"
|
||||
"{\n"
|
||||
" char prog[10];\n"
|
||||
" if (strlen(argv[0]) < 10)\n"
|
||||
" strcpy(prog, argv[0]);\n"
|
||||
"}\n");
|
||||
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("int main(int argc, char **argv, char **envp)\n"
|
||||
"{\n"
|
||||
" char prog[10] = {'\\0'};\n"
|
||||
" if (10 > strlen(argv[0]))\n"
|
||||
" strcat(prog, argv[0]);\n"
|
||||
"}\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"
|
||||
"}\n");
|
||||
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("int main(int argc, char **argv, char **envp)\n"
|
||||
"{\n"
|
||||
" char prog[10];\n"
|
||||
" argv[0][0] = '\\0';\n"
|
||||
" strcpy(prog, argv[0]);\n"
|
||||
"}\n");
|
||||
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
};
|
||||
|
||||
REGISTER_TEST(TestBufferOverrun)
|
||||
|
|
|
|||
Loading…
Reference in New Issue