From 93b3e2e5aafc9d7dbb621e870557c98533bb9d91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 18 Jul 2007 06:12:16 +0000 Subject: [PATCH] Buffer overruns, using string with unknown length --- CheckBufferOverrun.cpp | 120 ++++++++++++++++++++++ CommonCheck.cpp | 3 + main.cpp | 10 +- testbufferoverrun7/err.msg | 3 + testbufferoverrun7/testbufferoverrun7.cpp | 16 +++ 5 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 testbufferoverrun7/err.msg create mode 100644 testbufferoverrun7/testbufferoverrun7.cpp diff --git a/CheckBufferOverrun.cpp b/CheckBufferOverrun.cpp index 9620525be..a257563ab 100644 --- a/CheckBufferOverrun.cpp +++ b/CheckBufferOverrun.cpp @@ -9,6 +9,124 @@ //--------------------------------------------------------------------------- +TOKEN *findfunction(TOKEN *tok) +{ + int indentlevel = 0, parlevel = 0; + for (; tok; tok = tok->next) + { + if (tok->str[0] == '{') + indentlevel++; + else if (tok->str[0] == '}') + indentlevel--; + else if (tok->str[0] == '(') + parlevel++; + else if (tok->str[0] == ')') + parlevel--; + + if (!tok->next) + break; + + if (indentlevel==0 && parlevel==0 && IsName(tok->str) && tok->next->str[0]=='(') + { + for (TOKEN *tok2 = tok->next; tok2; tok2 = tok2->next) + { + if (tok2->str[0] == ')') + { + if (tok2->next->str[0] == '{') + return tok; + break; + } + } + } + } + + return 0; +} + + +//--------------------------------------------------------------------------- +// Writing dynamic data in buffer without bounds checking +//--------------------------------------------------------------------------- + +static void _DynamicDataCheck(TOKEN *ftok, TOKEN *tok) +{ + const char *var2 = tok->str; + bool decl = false; + unsigned int Var2Count = 0; + for ( TOKEN *tok2 = ftok; tok2; tok2 = tok2->next ) + { + if (tok2 == tok) + break; + if (match(tok2,"char * var")) + { + decl |= (strcmp(getstr(tok2,2),var2)==0); + tok2 = gettok(tok2,3); + if ( strcmp(tok2->str, "=") == 0 ) + { + Var2Count++; + break; + } + } + if (strcmp(tok2->str,var2)==0) + { + Var2Count++; + break; + } + } + + // The size of Var2 isn't checked, is it? + if (decl && Var2Count == 0) + { + std::ostringstream ostr; + ostr << FileLine(tok) << ": A string with unknown length is copied to buffer."; + ReportErr(ostr.str()); + } +} + + +static void _DynamicData() +{ + for (TOKEN *ftok = findfunction(tokens); ftok; ftok = findfunction(ftok->next)) + { + int indentlevel = 0; + for (TOKEN *tok = ftok; tok; tok = tok->next) + { + if (tok->str[0] == '{') + indentlevel++; + else if (tok->str[0] == '}') + { + indentlevel--; + if (indentlevel <= 0) + break; + } + + + if (match(tok,"strcpy ( var , var )") || + match(tok,"strcat ( var , var )") ) + { + _DynamicDataCheck(ftok,gettok(tok,4)); + } + + if (match(tok,"sprintf ( var")) + { + for ( TOKEN *tok2 = gettok(tok,3); tok2; tok2 = tok2->next ) + { + if (tok2->str[0] == ')') + break; + if (match(tok2,", var ,") || match(tok2,", var )")) + { + _DynamicDataCheck(ftok,tok2->next); + } + } + } + + } + } +} +//--------------------------------------------------------------------------- + + + //--------------------------------------------------------------------------- @@ -17,6 +135,8 @@ void CheckBufferOverrun() { + _DynamicData(); + int indentlevel = 0; for (TOKEN *tok = tokens; tok; tok = tok->next) { diff --git a/CommonCheck.cpp b/CommonCheck.cpp index 17a12be30..0ad4c71ec 100644 --- a/CommonCheck.cpp +++ b/CommonCheck.cpp @@ -4,6 +4,8 @@ #include #include //--------------------------------------------------------------------------- +bool HasErrors; +//--------------------------------------------------------------------------- std::string FileLine(TOKEN *tok) { @@ -16,6 +18,7 @@ std::string FileLine(TOKEN *tok) void ReportErr(const std::string errmsg) { std::cerr << errmsg << std::endl; + HasErrors = true; } //--------------------------------------------------------------------------- diff --git a/main.cpp b/main.cpp index 5a432cb1c..8d1ad497f 100644 --- a/main.cpp +++ b/main.cpp @@ -65,8 +65,12 @@ int main(int argc, char* argv[]) // CppCheck - A function that checks a specified file //--------------------------------------------------------------------------- +extern bool HasErrors; + static void CppCheck(const char FileName[]) { + HasErrors = false; + // Tokenize the file tokens = tokens_back = NULL; Files.clear(); @@ -74,7 +78,7 @@ static void CppCheck(const char FileName[]) // Check that the memsets are valid. - // This function can do dangerous things if used wrong. + // The 'memset' function can do dangerous things if used wrong. // Important: The checking doesn't work on simplified tokens list. CheckMemset(); @@ -149,6 +153,10 @@ static void CppCheck(const char FileName[]) // Clean up tokens.. DeallocateTokens(); + + // Todo: How should this work? Activated by a command line switch? + //if ( ! HasErrors ) + // std::cout << "No errors found\n"; } //--------------------------------------------------------------------------- diff --git a/testbufferoverrun7/err.msg b/testbufferoverrun7/err.msg new file mode 100644 index 000000000..5ea0a6270 --- /dev/null +++ b/testbufferoverrun7/err.msg @@ -0,0 +1,3 @@ +[testbufferoverrun7\testbufferoverrun7.cpp:5]: A string with unknown length is copied to buffer. +[testbufferoverrun7\testbufferoverrun7.cpp:10]: A string with unknown length is copied to buffer. +[testbufferoverrun7\testbufferoverrun7.cpp:15]: A string with unknown length is copied to buffer. diff --git a/testbufferoverrun7/testbufferoverrun7.cpp b/testbufferoverrun7/testbufferoverrun7.cpp new file mode 100644 index 000000000..56f36eda1 --- /dev/null +++ b/testbufferoverrun7/testbufferoverrun7.cpp @@ -0,0 +1,16 @@ + + +void f1(char *str) +{ + strcpy(buf,str); +} + +void f2(char *str) +{ + strcat(buf,str); +} + +void f3(char *str) +{ + sprintf(buf,"%s",str); +}