From b44029cdaa4d007181374ad0002dacbd792f7e16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 1 Jan 2020 09:09:10 +0100 Subject: [PATCH] Refactoring; CWEs should be clarified --- lib/checknullpointer.cpp | 23 ++++++++++------------- lib/checkuninitvar.cpp | 18 ++++++------------ lib/errorlogger.h | 6 ++++++ lib/exprengine.cpp | 2 +- 4 files changed, 23 insertions(+), 26 deletions(-) diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index e6750cecc..fa87c231a 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -40,9 +40,6 @@ namespace { CheckNullPointer instance; } -static const CWE CWE476(476U); // NULL Pointer Dereference -static const CWE CWE682(682U); // Incorrect Calculation - //--------------------------------------------------------------------------- static bool checkNullpointerFunctionCallPlausibility(const Function* func, unsigned int arg) @@ -487,14 +484,14 @@ void CheckNullPointer::nullPointerError(const Token *tok, const std::string &var const std::string errmsgdefarg("$symbol:" + varname + "\nPossible null pointer dereference if the default parameter value is used: $symbol"); if (!tok) { - reportError(tok, Severity::error, "nullPointer", "Null pointer dereference", CWE476, false); - reportError(tok, Severity::warning, "nullPointerDefaultArg", errmsgdefarg, CWE476, false); - reportError(tok, Severity::warning, "nullPointerRedundantCheck", errmsgcond, CWE476, false); + reportError(tok, Severity::error, "nullPointer", "Null pointer dereference", CWE_NULL_POINTER_DEREFERENCE, false); + reportError(tok, Severity::warning, "nullPointerDefaultArg", errmsgdefarg, CWE_NULL_POINTER_DEREFERENCE, false); + reportError(tok, Severity::warning, "nullPointerRedundantCheck", errmsgcond, CWE_NULL_POINTER_DEREFERENCE, false); return; } if (!value) { - reportError(tok, Severity::error, "nullPointer", "Null pointer dereference", CWE476, inconclusive); + reportError(tok, Severity::error, "nullPointer", "Null pointer dereference", CWE_NULL_POINTER_DEREFERENCE, inconclusive); return; } @@ -504,9 +501,9 @@ void CheckNullPointer::nullPointerError(const Token *tok, const std::string &var const ErrorPath errorPath = getErrorPath(tok, value, "Null pointer dereference"); if (value->condition) { - reportError(errorPath, Severity::warning, "nullPointerRedundantCheck", errmsgcond, CWE476, inconclusive || value->isInconclusive()); + reportError(errorPath, Severity::warning, "nullPointerRedundantCheck", errmsgcond, CWE_NULL_POINTER_DEREFERENCE, inconclusive || value->isInconclusive()); } else if (value->defaultArg) { - reportError(errorPath, Severity::warning, "nullPointerDefaultArg", errmsgdefarg, CWE476, inconclusive || value->isInconclusive()); + reportError(errorPath, Severity::warning, "nullPointerDefaultArg", errmsgdefarg, CWE_NULL_POINTER_DEREFERENCE, inconclusive || value->isInconclusive()); } else { std::string errmsg; errmsg = std::string(value->isKnown() ? "Null" : "Possible null") + " pointer dereference"; @@ -517,7 +514,7 @@ void CheckNullPointer::nullPointerError(const Token *tok, const std::string &var value->isKnown() ? Severity::error : Severity::warning, "nullPointer", errmsg, - CWE476, inconclusive || value->isInconclusive()); + CWE_NULL_POINTER_DEREFERENCE, inconclusive || value->isInconclusive()); } } @@ -590,7 +587,7 @@ void CheckNullPointer::pointerArithmeticError(const Token* tok, const ValueFlow: Severity::error, "nullPointerArithmetic", errmsg, - CWE682, + CWE_INCORRECT_CALCULATION, inconclusive); } @@ -608,7 +605,7 @@ void CheckNullPointer::redundantConditionWarning(const Token* tok, const ValueFl Severity::warning, "nullPointerArithmeticRedundantCheck", errmsg, - CWE682, + CWE_INCORRECT_CALCULATION, inconclusive); } @@ -681,7 +678,7 @@ bool CheckNullPointer::analyseWholeProgram(const CTU::FileInfo *ctu, const std:: warning ? Severity::warning : Severity::error, "Null pointer dereference: " + unsafeUsage.myArgumentName, "ctunullpointer", - CWE476, false); + CWE_NULL_POINTER_DEREFERENCE, false); errorLogger.reportErr(errmsg); foundErrors = true; diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 644836cad..e82ffc32a 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -48,12 +48,6 @@ namespace { //--------------------------------------------------------------------------- -// CWE ids used: -static const struct CWE CWE476(476U); // NULL Pointer Dereference -static const struct CWE CWE676(676U); -static const struct CWE CWE908(908U); -static const struct CWE CWE825(825U); - // get ast parent, skip possible address-of and casts static const Token *getAstParentSkipPossibleCastAndAddressOf(const Token *vartok, bool *unknown) { @@ -1297,19 +1291,18 @@ bool CheckUninitVar::isMemberVariableUsage(const Token *tok, bool isPointer, All void CheckUninitVar::uninitstringError(const Token *tok, const std::string &varname, bool strncpy_) { - reportError(tok, Severity::error, "uninitstring", "$symbol:" + varname + "\nDangerous usage of '$symbol'" + (strncpy_ ? " (strncpy doesn't always null-terminate it)." : " (not null-terminated)."), CWE676, false); + reportError(tok, Severity::error, "uninitstring", "$symbol:" + varname + "\nDangerous usage of '$symbol'" + (strncpy_ ? " (strncpy doesn't always null-terminate it)." : " (not null-terminated)."), CWE_USE_OF_POTENTIALLY_DANGEROUS_FUNCTION, false); } void CheckUninitVar::uninitdataError(const Token *tok, const std::string &varname) { - reportError(tok, Severity::error, "uninitdata", "$symbol:" + varname + "\nMemory is allocated but not initialized: $symbol", CWE908, false); + reportError(tok, Severity::error, "uninitdata", "$symbol:" + varname + "\nMemory is allocated but not initialized: $symbol", CWE_USE_OF_UNINITIALIZED_VARIABLE, false); } void CheckUninitVar::uninitvarError(const Token *tok, const std::string &varname, ErrorPath errorPath) { errorPath.emplace_back(tok, ""); - reportError(errorPath, Severity::error, "uninitvar", "$symbol:" + varname + "\nUninitialized variable: $symbol", CWE908, false); - // reportError(tok, Severity::error, "uninitvar", "$symbol:" + varname + "\nUninitialized variable: $symbol", CWE908, false); + reportError(errorPath, Severity::error, "uninitvar", "$symbol:" + varname + "\nUninitialized variable: $symbol", CWE_USE_OF_UNINITIALIZED_VARIABLE, false); } void CheckUninitVar::uninitStructMemberError(const Token *tok, const std::string &membername) @@ -1317,7 +1310,7 @@ void CheckUninitVar::uninitStructMemberError(const Token *tok, const std::string reportError(tok, Severity::error, "uninitStructMember", - "$symbol:" + membername + "\nUninitialized struct member: $symbol", CWE908, false); + "$symbol:" + membername + "\nUninitialized struct member: $symbol", CWE_USE_OF_UNINITIALIZED_VARIABLE, false); } static bool isLeafDot(const Token* tok) @@ -1452,7 +1445,8 @@ bool CheckUninitVar::analyseWholeProgram(const CTU::FileInfo *ctu, const std::li Severity::error, "Using argument " + unsafeUsage.myArgumentName + " that points at uninitialized variable " + functionCall->callArgumentExpression, "ctuuninitvar", - CWE908, false); + CWE_USE_OF_UNINITIALIZED_VARIABLE, + false); errorLogger.reportErr(errmsg); foundErrors = true; diff --git a/lib/errorlogger.h b/lib/errorlogger.h index 6d2af87fb..6ce686180 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -40,6 +40,12 @@ struct CWE { unsigned short id; }; +// CWE list: https://cwe.mitre.org/data/published/cwe_v3.4.1.pdf +static const struct CWE CWE_USE_OF_UNINITIALIZED_VARIABLE(457U); +static const struct CWE CWE_NULL_POINTER_DEREFERENCE(476U); +static const struct CWE CWE_USE_OF_POTENTIALLY_DANGEROUS_FUNCTION(676U); +static const struct CWE CWE_INCORRECT_CALCULATION(682U); +static const struct CWE CWE_EXPIRED_POINTER_DEREFERENCE(825U); class Token; diff --git a/lib/exprengine.cpp b/lib/exprengine.cpp index 3f7fd5209..02be70dc6 100644 --- a/lib/exprengine.cpp +++ b/lib/exprengine.cpp @@ -1848,7 +1848,7 @@ void ExprEngine::runChecks(ErrorLogger *errorLogger, const Tokenizer *tokenizer, dataBase->addError(tok->linenr()); std::list callstack{tok}; - ErrorLogger::ErrorMessage errmsg(callstack, &tokenizer->list, Severity::SeverityType::error, "verificationUninit", "Cannot determine that '" + tok->expressionString() + "' is initialized", CWE(908), false); + ErrorLogger::ErrorMessage errmsg(callstack, &tokenizer->list, Severity::SeverityType::error, "verificationUninit", "Cannot determine that '" + tok->expressionString() + "' is initialized", CWE_USE_OF_UNINITIALIZED_VARIABLE, false); errorLogger->reportErr(errmsg); }; #endif