From d3645d874e98d715674f002442f855972434dc7c Mon Sep 17 00:00:00 2001 From: Roberto Martelloni Date: Sat, 20 Feb 2016 22:56:36 +0000 Subject: [PATCH] Mapped toomanyconfigs ,AssignmentAddressToInteger ,AssignmentIntegerToAddress ,CastIntegerToAddressAtReturn ,CastAddressToIntegerAtReturn ,assertWithSideEffect ,assignmentInAssert ,uselessAssignmentArg ,uselessAssignmentPtrArg ,comparisonOfFuncReturningBoolError ,comparisonOfTwoFuncsReturningBoolError ,comparisonOfBoolWithBoolError ,incrementboolean ,comparisonOfBoolWithInt ,compareBoolExpressionWithInt ,negativeIndex ,pointerOutOfBounds ,arrayIndexThenCheck ,possibleBufferAccessOutOfBounds ,argumentSize ,arrayIndexOutOfBoundsCond ,noConstructor ,copyCtorPointerCopying ,noCopyConstructor ,uninitMemberVar ,operatorEqVarError ,unusedPrivateFunction ,memsetClassFloat ,mallocOnClassWarning ,operatorEq ,thisSubtraction ,operatorEqRetRefThis ,operatorEqToSelf ,useInitializationList ,duplInheritedMember ,assignIfError ,comparisonError ,multiCondition ,mismatchingBitAnd ,oppositeInnerCondition ,incorrectLogicOperator ,redundantCondition ,moduloAlwaysTrueFalse to their CWEs ids. --- lib/check.h | 6 +----- lib/check64bit.cpp | 12 ++++++++---- lib/checkassert.cpp | 6 ++++-- lib/checkautovariables.cpp | 9 +++++---- lib/checkbool.cpp | 20 +++++++++++--------- lib/checkbufferoverrun.cpp | 23 ++++++++++++----------- lib/checkbufferoverrun.h | 6 +++++- lib/checkclass.cpp | 35 +++++++++++++++++++---------------- lib/checkcondition.cpp | 25 +++++++++++++++---------- lib/cppcheck.cpp | 5 ++++- lib/errorlogger.cpp | 32 ++++++++++++++++++++++++++++++++ lib/errorlogger.h | 9 +++++++++ 12 files changed, 125 insertions(+), 63 deletions(-) diff --git a/lib/check.h b/lib/check.h index 7ad86245c..8c7287987 100644 --- a/lib/check.h +++ b/lib/check.h @@ -33,10 +33,6 @@ /// @addtogroup Core /// @{ -struct CWE { - explicit CWE(unsigned short ID) : id(ID) {} - unsigned short id; -}; /** * @brief Interface class that cppcheck uses to communicate with the checks. @@ -135,7 +131,7 @@ protected: /** report an error */ template void reportError(const std::list &callstack, Severity::SeverityType severity, const T id, const U msg, const CWE &cwe, bool inconclusive) { - ErrorLogger::ErrorMessage errmsg(callstack, _tokenizer?&_tokenizer->list:0, severity, id, msg, inconclusive); + ErrorLogger::ErrorMessage errmsg(callstack, _tokenizer?&_tokenizer->list:0, severity, id, msg, cwe, inconclusive); errmsg._cwe = cwe.id; if (_errorLogger) _errorLogger->reportErr(errmsg); diff --git a/lib/check64bit.cpp b/lib/check64bit.cpp index 3e1c9ff47..144115c5d 100644 --- a/lib/check64bit.cpp +++ b/lib/check64bit.cpp @@ -25,6 +25,10 @@ //--------------------------------------------------------------------------- +// CWE ids used +static const struct CWE CWE398(398U); // Indicator of Poor Code Quality +static const struct CWE CWE758(758U); // Reliance on Undefined, Unspecified, or Implementation-Defined Behavior + // Register this check class (by creating a static instance of it) namespace { Check64BitPortability instance; @@ -115,7 +119,7 @@ void Check64BitPortability::assignmentAddressToIntegerError(const Token *tok) "Assigning a pointer to an integer (int/long/etc) is not portable across different platforms and " "compilers. For example in 32-bit Windows and linux they are same width, but in 64-bit Windows and linux " "they are of different width. In worst case you end up assigning 64-bit address to 32-bit integer. The safe " - "way is to store addresses only in pointer types (or typedefs like uintptr_t)."); + "way is to store addresses only in pointer types (or typedefs like uintptr_t).", CWE758, false); } void Check64BitPortability::assignmentIntegerToAddressError(const Token *tok) @@ -126,7 +130,7 @@ void Check64BitPortability::assignmentIntegerToAddressError(const Token *tok) "Assigning an integer (int/long/etc) to a pointer is not portable across different platforms and " "compilers. For example in 32-bit Windows and linux they are same width, but in 64-bit Windows and linux " "they are of different width. In worst case you end up assigning 64-bit integer to 32-bit pointer. The safe " - "way is to store addresses only in pointer types (or typedefs like uintptr_t)."); + "way is to store addresses only in pointer types (or typedefs like uintptr_t).", CWE758, false); } void Check64BitPortability::returnPointerError(const Token *tok) @@ -137,7 +141,7 @@ void Check64BitPortability::returnPointerError(const Token *tok) "Returning an address value in a function with integer (int/long/etc) return type is not portable across " "different platforms and compilers. For example in 32-bit Windows and Linux they are same width, but in " "64-bit Windows and Linux they are of different width. In worst case you end up casting 64-bit address down " - "to 32-bit integer. The safe way is to always return an integer."); + "to 32-bit integer. The safe way is to always return an integer.", CWE758, false); } void Check64BitPortability::returnIntegerError(const Token *tok) @@ -148,5 +152,5 @@ void Check64BitPortability::returnIntegerError(const Token *tok) "Returning an integer (int/long/etc) in a function with pointer return type is not portable across different " "platforms and compilers. For example in 32-bit Windows and Linux they are same width, but in 64-bit Windows " "and Linux they are of different width. In worst case you end up casting 64-bit integer down to 32-bit pointer. " - "The safe way is to always return a pointer."); + "The safe way is to always return a pointer.", CWE758, false); } diff --git a/lib/checkassert.cpp b/lib/checkassert.cpp index c9238adef..52bfb7bfa 100644 --- a/lib/checkassert.cpp +++ b/lib/checkassert.cpp @@ -25,6 +25,8 @@ //--------------------------------------------------------------------------- +// CWE ids used +static const struct CWE CWE398(398U); // Indicator of Poor Code Quality // Register this check class (by creating a static instance of it) namespace { @@ -92,7 +94,7 @@ void CheckAssert::sideEffectInAssertError(const Token *tok, const std::string& f "Non-pure function: '" + functionName + "' is called inside assert statement. " "Assert statements are removed from release builds so the code inside " "assert statement is not executed. If the code is needed also in release " - "builds, this is a bug."); + "builds, this is a bug.", CWE398, false); } void CheckAssert::assignmentInAssertError(const Token *tok, const std::string& varname) @@ -102,7 +104,7 @@ void CheckAssert::assignmentInAssertError(const Token *tok, const std::string& v "Variable '" + varname + "' is modified insert assert statement. " "Assert statements are removed from release builds so the code inside " "assert statement is not executed. If the code is needed also in release " - "builds, this is a bug."); + "builds, this is a bug.", CWE398, false); } // checks if side effects happen on the variable prior to tmp diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index dd339a8c7..bd1b7e982 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -33,8 +33,9 @@ namespace { static CheckAutoVariables instance; } -static const CWE CWE562(562U); -static const CWE CWE590(590U); +static const CWE CWE398(398U); // Indicator of Poor Code Quality +static const CWE CWE562(562U); // Return of Stack Variable Address +static const CWE CWE590(590U); // Free of Memory not on the Heap bool CheckAutoVariables::isPtrArg(const Token *tok) { @@ -332,7 +333,7 @@ void CheckAutoVariables::errorUselessAssignmentArg(const Token *tok) reportError(tok, Severity::style, "uselessAssignmentArg", - "Assignment of function parameter has no effect outside the function."); + "Assignment of function parameter has no effect outside the function.", CWE398, false); } void CheckAutoVariables::errorUselessAssignmentPtrArg(const Token *tok) @@ -340,7 +341,7 @@ void CheckAutoVariables::errorUselessAssignmentPtrArg(const Token *tok) reportError(tok, Severity::warning, "uselessAssignmentPtrArg", - "Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?"); + "Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?", CWE398, false); } //--------------------------------------------------------------------------- diff --git a/lib/checkbool.cpp b/lib/checkbool.cpp index 5d1cd9cea..87bd0cf4f 100644 --- a/lib/checkbool.cpp +++ b/lib/checkbool.cpp @@ -29,8 +29,9 @@ namespace { CheckBool instance; } -static const CWE CWE571(571); -static const CWE CWE587(587); +static const CWE CWE398(398U); // Indicator of Poor Code Quality +static const CWE CWE571(571U); // Expression is Always True +static const CWE CWE587(587U); // Assignment of a Fixed Address to a Pointer //--------------------------------------------------------------------------- //--------------------------------------------------------------------------- @@ -60,7 +61,8 @@ void CheckBool::incrementBooleanError(const Token *tok) Severity::style, "incrementboolean", "Incrementing a variable of type 'bool' with postfix operator++ is deprecated by the C++ Standard. You should assign it the value 'true' instead.\n" - "The operand of a postfix increment operator may be of type bool but it is deprecated by C++ Standard (Annex D-1) and the operand is always set to true. You should assign it the value 'true' instead." + "The operand of a postfix increment operator may be of type bool but it is deprecated by C++ Standard (Annex D-1) and the operand is always set to true. You should assign it the value 'true' instead.", + CWE398, false ); } @@ -104,7 +106,7 @@ void CheckBool::bitwiseOnBooleanError(const Token *tok, const std::string &varna { reportError(tok, Severity::style, "bitwiseOnBoolean", "Boolean variable '" + varname + "' is used in bitwise operation. Did you mean '" + op + "'?", - CWE(0), + CWE398, true); } @@ -214,7 +216,7 @@ void CheckBool::comparisonOfFuncReturningBoolError(const Token *tok, const std:: "Comparison of a function returning boolean value using relational (<, >, <= or >=) operator.\n" "The return type of function '" + expression + "' is 'bool' " "and result is of type 'bool'. Comparing 'bool' value using relational (<, >, <= or >=)" - " operator could cause unexpected results."); + " operator could cause unexpected results.", CWE398, false); } void CheckBool::comparisonOfTwoFuncsReturningBoolError(const Token *tok, const std::string &expression1, const std::string &expression2) @@ -223,7 +225,7 @@ void CheckBool::comparisonOfTwoFuncsReturningBoolError(const Token *tok, const s "Comparison of two functions returning boolean value using relational (<, >, <= or >=) operator.\n" "The return type of function '" + expression1 + "' and function '" + expression2 + "' is 'bool' " "and result is of type 'bool'. Comparing 'bool' value using relational (<, >, <= or >=)" - " operator could cause unexpected results."); + " operator could cause unexpected results.", CWE398, false); } //------------------------------------------------------------------------------- @@ -282,7 +284,7 @@ void CheckBool::comparisonOfBoolWithBoolError(const Token *tok, const std::strin "Comparison of a variable having boolean value using relational (<, >, <= or >=) operator.\n" "The variable '" + expression + "' is of type 'bool' " "and comparing 'bool' value using relational (<, >, <= or >=)" - " operator could cause unexpected results."); + " operator could cause unexpected results.", CWE398, false); } //----------------------------------------------------------------------------- @@ -373,10 +375,10 @@ void CheckBool::comparisonOfBoolExpressionWithIntError(const Token *tok, bool n0 { if (n0o1) reportError(tok, Severity::warning, "compareBoolExpressionWithInt", - "Comparison of a boolean expression with an integer other than 0 or 1."); + "Comparison of a boolean expression with an integer other than 0 or 1.", CWE398, false); else reportError(tok, Severity::warning, "compareBoolExpressionWithInt", - "Comparison of a boolean expression with an integer."); + "Comparison of a boolean expression with an integer.", CWE398, false); } diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index c7abee0cb..63f388422 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -44,8 +44,9 @@ namespace { //--------------------------------------------------------------------------- // CWE ids used: -static const CWE CWE119(119U); static const CWE CWE131(131U); +static const CWE CWE398(398U); +static const CWE CWE786(786U); static const CWE CWE788(788U); //--------------------------------------------------------------------------- @@ -99,7 +100,7 @@ void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const Arra std::list callstack; callstack.push_back(tok); callstack.push_back(condition); - reportError(callstack, Severity::warning, "arrayIndexOutOfBoundsCond", errmsg.str(), CWE(0U), false); + reportError(callstack, Severity::warning, "arrayIndexOutOfBoundsCond", errmsg.str(), CWE119, false); } else { std::ostringstream errmsg; errmsg << "Array '" << arrayInfo.varname(); @@ -114,7 +115,7 @@ void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const Arra errmsg << " out of bounds."; } - reportError(tok, Severity::error, "arrayIndexOutOfBounds", errmsg.str()); + reportError(tok, Severity::error, "arrayIndexOutOfBounds", errmsg.str(), CWE119, false); } } @@ -155,12 +156,12 @@ void CheckBufferOverrun::possibleBufferOverrunError(const Token *tok, const std: reportError(tok, Severity::warning, "possibleBufferAccessOutOfBounds", "Possible buffer overflow if strlen(" + src + ") is larger than sizeof(" + dst + ")-strlen(" + dst +").\n" "Possible buffer overflow if strlen(" + src + ") is larger than sizeof(" + dst + ")-strlen(" + dst +"). " - "The source buffer is larger than the destination buffer so there is the potential for overflowing the destination buffer."); + "The source buffer is larger than the destination buffer so there is the potential for overflowing the destination buffer.", CWE398, false); else reportError(tok, Severity::warning, "possibleBufferAccessOutOfBounds", "Possible buffer overflow if strlen(" + src + ") is larger than or equal to sizeof(" + dst + ").\n" "Possible buffer overflow if strlen(" + src + ") is larger than or equal to sizeof(" + dst + "). " - "The source buffer is larger than the destination buffer so there is the potential for overflowing the destination buffer."); + "The source buffer is larger than the destination buffer so there is the potential for overflowing the destination buffer.", CWE398, false); } void CheckBufferOverrun::strncatUsageError(const Token *tok) @@ -205,7 +206,7 @@ void CheckBufferOverrun::pointerOutOfBoundsError(const Token *tok, const Token * } std::string verbosemsg(errmsg + ". From chapter 6.5.6 in the C specification:\n" "\"When an expression that has integer type is added to or subtracted from a pointer, ..\" and then \"If both the pointer operand and the result point to elements of the same array object, or one past the last element of the array object, the evaluation shall not produce an overflow; otherwise, the behavior is undefined.\""); - reportError(tok, Severity::portability, "pointerOutOfBounds", errmsg + ".\n" + verbosemsg); + reportError(tok, Severity::portability, "pointerOutOfBounds", errmsg + ".\n" + verbosemsg, CWE398, false); /* "Undefined behaviour: The result of this pointer arithmetic does not point into or just one element past the end of the " + object + ". @@ -247,7 +248,7 @@ void CheckBufferOverrun::bufferNotZeroTerminatedError(const Token *tok, const st void CheckBufferOverrun::argumentSizeError(const Token *tok, const std::string &functionName, const std::string &varname) { - reportError(tok, Severity::warning, "argumentSize", "The array '" + varname + "' is too small, the function '" + functionName + "' expects a bigger one."); + reportError(tok, Severity::warning, "argumentSize", "The array '" + varname + "' is too small, the function '" + functionName + "' expects a bigger one.", CWE398, false); } void CheckBufferOverrun::negativeMemoryAllocationSizeError(const Token *tok) @@ -1727,7 +1728,7 @@ void CheckBufferOverrun::negativeIndexError(const Token *tok, MathLib::bigint in { std::ostringstream ostr; ostr << "Array index " << index << " is out of bounds."; - reportError(tok, Severity::error, "negativeIndex", ostr.str()); + reportError(tok, Severity::error, "negativeIndex", ostr.str(), CWE786, false); } void CheckBufferOverrun::negativeIndexError(const Token *tok, const ValueFlow::Value &index) @@ -1736,7 +1737,7 @@ void CheckBufferOverrun::negativeIndexError(const Token *tok, const ValueFlow::V ostr << "Array index " << index.intvalue << " is out of bounds."; if (index.condition) ostr << " Otherwise there is useless condition at line " << index.condition->linenr() << "."; - reportError(tok, index.condition ? Severity::warning : Severity::error, "negativeIndex", ostr.str(), CWE(0U), index.inconclusive); + reportError(tok, index.condition ? Severity::warning : Severity::error, "negativeIndex", ostr.str(), CWE786, index.inconclusive); } CheckBufferOverrun::ArrayInfo::ArrayInfo() @@ -1870,7 +1871,7 @@ void CheckBufferOverrun::arrayIndexThenCheckError(const Token *tok, const std::s "Defensive programming: The variable '" + indexName + "' is used as an array index before it " "is checked that is within limits. This can mean that the array might be accessed out of bounds. " "Reorder conditions such as '(a[i] && i < 10)' to '(i < 10 && a[i])'. That way the array will " - "not be accessed if the index is out of limits."); + "not be accessed if the index is out of limits.", CWE398, false); } Check::FileInfo* CheckBufferOverrun::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const @@ -1962,7 +1963,7 @@ void CheckBufferOverrun::analyseWholeProgram(const std::list & Severity::error, ostr.str(), "arrayIndexOutOfBounds", - false); + CWE788, false); errorLogger.reportErr(errmsg); } } diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index e8fc8295a..3029c2662 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -25,10 +25,14 @@ #include "config.h" #include "check.h" #include "mathlib.h" + #include #include #include +// CWE ids used +static const struct CWE CWE119(119U); // Improper Restriction of Operations within the Bounds of a Memory Buffer + class Variable; /// @addtogroup Checks @@ -256,7 +260,7 @@ public: c.argumentSizeError(0, "function", "array"); c.negativeMemoryAllocationSizeError(0); c.negativeArraySizeError(0); - c.reportError(nullptr, Severity::warning, "arrayIndexOutOfBoundsCond", "Array 'x[10]' accessed at index 20, which is out of bounds. Otherwise condition 'y==20' is redundant."); + c.reportError(nullptr, Severity::warning, "arrayIndexOutOfBoundsCond", "Array 'x[10]' accessed at index 20, which is out of bounds. Otherwise condition 'y==20' is redundant.", CWE119, false); } private: diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 63675beb6..e0a7b6c6b 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -35,8 +35,10 @@ namespace { CheckClass instance; } +static const CWE CWE398(398U); static const CWE CWE404(404U); static const CWE CWE665(665U); +static const CWE CWE758(758U); static const CWE CWE762(762U); static const char * getFunctionTypeName( @@ -362,7 +364,8 @@ void CheckClass::copyConstructorMallocError(const Token *cctor, const Token *all void CheckClass::copyConstructorShallowCopyError(const Token *tok, const std::string& varname) { - reportError(tok, Severity::style, "copyCtorPointerCopying", "Value of pointer '" + varname + "', which points to allocated memory, is copied in copy constructor instead of allocating new memory."); + reportError(tok, Severity::style, "copyCtorPointerCopying", + "Value of pointer '" + varname + "', which points to allocated memory, is copied in copy constructor instead of allocating new memory.", CWE398, false); } void CheckClass::noCopyConstructorError(const Token *tok, const std::string &classname, bool isStruct) @@ -370,7 +373,7 @@ void CheckClass::noCopyConstructorError(const Token *tok, const std::string &cla // The constructor might be intentionally missing. Therefore this is not a "warning" reportError(tok, Severity::style, "noCopyConstructor", "'" + std::string(isStruct ? "struct" : "class") + " " + classname + - "' does not have a copy constructor which is recommended since the class contains a pointer to allocated memory."); + "' does not have a copy constructor which is recommended since the class contains a pointer to allocated memory.", CWE398, false); } bool CheckClass::canNotCopy(const Scope *scope) @@ -780,7 +783,7 @@ void CheckClass::noConstructorError(const Token *tok, const std::string &classna "The " + std::string(isStruct ? "struct" : "class") + " '" + classname + "' does not have a constructor although it has private member variables. " "Member variables of builtin types are left uninitialized when the class is " - "instantiated. That may cause bugs or undefined behavior."); + "instantiated. That may cause bugs or undefined behavior.", CWE398, false); } void CheckClass::noExplicitConstructorError(const Token *tok, const std::string &classname, bool isStruct) @@ -792,12 +795,12 @@ void CheckClass::noExplicitConstructorError(const Token *tok, const std::string void CheckClass::uninitVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive) { - reportError(tok, Severity::warning, "uninitMemberVar", "Member variable '" + classname + "::" + varname + "' is not initialized in the constructor.", CWE(0U), inconclusive); + reportError(tok, Severity::warning, "uninitMemberVar", "Member variable '" + classname + "::" + varname + "' is not initialized in the constructor.", CWE398, inconclusive); } void CheckClass::operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive) { - reportError(tok, Severity::warning, "operatorEqVarError", "Member variable '" + classname + "::" + varname + "' is not assigned a value in '" + classname + "::operator='.", CWE(0U), inconclusive); + reportError(tok, Severity::warning, "operatorEqVarError", "Member variable '" + classname + "::" + varname + "' is not assigned a value in '" + classname + "::operator='.", CWE398, inconclusive); } //--------------------------------------------------------------------------- @@ -863,7 +866,7 @@ void CheckClass::suggestInitializationList(const Token* tok, const std::string& reportError(tok, Severity::performance, "useInitializationList", "Variable '" + varname + "' is assigned in constructor body. Consider performing initialization in initialization list.\n" "When an object of a class is created, the constructors of all member variables are called consecutively " "in the order the variables are declared, even if you don't explicitly write them to the initialization list. You " - "could avoid assigning '" + varname + "' a value by passing the value to the constructor in the initialization list."); + "could avoid assigning '" + varname + "' a value by passing the value to the constructor in the initialization list.", CWE398, false); } //--------------------------------------------------------------------------- @@ -971,7 +974,7 @@ void CheckClass::privateFunctions() void CheckClass::unusedPrivateFunctionError(const Token *tok, const std::string &classname, const std::string &funcname) { - reportError(tok, Severity::style, "unusedPrivateFunction", "Unused private function: '" + classname + "::" + funcname + "'"); + reportError(tok, Severity::style, "unusedPrivateFunction", "Unused private function: '" + classname + "::" + funcname + "'", CWE398, false); } //--------------------------------------------------------------------------- @@ -1137,7 +1140,7 @@ void CheckClass::mallocOnClassWarning(const Token* tok, const std::string &memfu reportError(toks, Severity::warning, "mallocOnClassWarning", "Memory for class instance allocated with " + memfunc + "(), but class provides constructors.\n" "Memory for class instance allocated with " + memfunc + "(), but class provides constructors. This is unsafe, " - "since no constructor is called and class members remain uninitialized. Consider using 'new' instead.", CWE(0U), false); + "since no constructor is called and class members remain uninitialized. Consider using 'new' instead.", CWE762, false); } void CheckClass::mallocOnClassError(const Token* tok, const std::string &memfunc, const Token* classTok, const std::string &classname) @@ -1171,7 +1174,7 @@ void CheckClass::memsetErrorFloat(const Token *tok, const std::string &type) "Using memset() on " + type + " which contains a floating point number." " This is not portable because memset() sets each byte of a block of memory to a specific value and" " the actual representation of a floating-point value is implementation defined." - " Note: In case of an IEEE754-1985 compatible implementation setting all bits to zero results in the value 0.0."); + " Note: In case of an IEEE754-1985 compatible implementation setting all bits to zero results in the value 0.0.", CWE758, false); } @@ -1225,7 +1228,7 @@ void CheckClass::operatorEqReturnError(const Token *tok, const std::string &clas { reportError(tok, Severity::style, "operatorEq", "'" + className + "::operator=' should return '" + className + " &'.\n" "The "+className+"::operator= does not conform to standard C/C++ behaviour. To conform to standard C/C++ behaviour, return a reference to self (such as: '"+className+" &"+className+"::operator=(..) { .. return *this; }'. For safety reasons it might be better to not fix this message. If you think that safety is always more important than conformance then please ignore/suppress this message. For more details about this topic, see the book \"Effective C++\" by Scott Meyers." - ); + , CWE398, false); } //--------------------------------------------------------------------------- @@ -1338,18 +1341,18 @@ void CheckClass::checkReturnPtrThis(const Scope *scope, const Function *func, co void CheckClass::operatorEqRetRefThisError(const Token *tok) { - reportError(tok, Severity::style, "operatorEqRetRefThis", "'operator=' should return reference to 'this' instance."); + reportError(tok, Severity::style, "operatorEqRetRefThis", "'operator=' should return reference to 'this' instance.", CWE398, false); } void CheckClass::operatorEqShouldBeLeftUnimplementedError(const Token *tok) { - reportError(tok, Severity::style, "operatorEqShouldBeLeftUnimplemented", "'operator=' should either return reference to 'this' instance or be declared private and left unimplemented."); + reportError(tok, Severity::style, "operatorEqShouldBeLeftUnimplemented", "'operator=' should either return reference to 'this' instance or be declared private and left unimplemented.", CWE398, false); } void CheckClass::operatorEqMissingReturnStatementError(const Token *tok, bool error) { if (error) { - reportError(tok, Severity::error, "operatorEqMissingReturnStatement", "No 'return' statement in non-void function causes undefined behavior."); + reportError(tok, Severity::error, "operatorEqMissingReturnStatement", "No 'return' statement in non-void function causes undefined behavior.", CWE398, false); } else { operatorEqRetRefThisError(tok); } @@ -1473,7 +1476,7 @@ void CheckClass::operatorEqToSelfError(const Token *tok) reportError(tok, Severity::warning, "operatorEqToSelf", "'operator=' should check for assignment to self to avoid problems with dynamic memory.\n" "'operator=' should check for assignment to self to ensure that each block of dynamically " - "allocated memory is owned and managed by only one instance of the class."); + "allocated memory is owned and managed by only one instance of the class.", CWE398, false); } //--------------------------------------------------------------------------- @@ -1651,7 +1654,7 @@ void CheckClass::thisSubtraction() void CheckClass::thisSubtractionError(const Token *tok) { - reportError(tok, Severity::warning, "thisSubtraction", "Suspicious pointer subtraction. Did you intend to write '->'?"); + reportError(tok, Severity::warning, "thisSubtraction", "Suspicious pointer subtraction. Did you intend to write '->'?", CWE398, false); } //--------------------------------------------------------------------------- @@ -2293,5 +2296,5 @@ void CheckClass::duplInheritedMembersError(const Token *tok1, const Token* tok2, const std::string message = "The " + std::string(derivedIsStruct ? "struct" : "class") + " '" + derivedname + "' defines member variable with name '" + variablename + "' also defined in its parent " + std::string(baseIsStruct ? "struct" : "class") + " '" + basename + "'."; - reportError(toks, Severity::warning, "duplInheritedMember", message, CWE(0U), false); + reportError(toks, Severity::warning, "duplInheritedMember", message, CWE398, false); } diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 353721471..4443d2723 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -28,6 +28,11 @@ #include #include +// CWE ids used +static const struct CWE CWE398(398U); +static const struct CWE CWE570(570U); +static const struct CWE CWE571(571U); + //--------------------------------------------------------------------------- // Register this check class (by creating a static instance of it) @@ -191,7 +196,7 @@ void CheckCondition::assignIfError(const Token *tok1, const Token *tok2, const s reportError(locations, Severity::style, "assignIfError", - "Mismatching assignment and comparison, comparison '" + condition + "' is always " + std::string(result ? "true" : "false") + "."); + "Mismatching assignment and comparison, comparison '" + condition + "' is always " + std::string(result ? "true" : "false") + ".", CWE398, false); } @@ -208,7 +213,7 @@ void CheckCondition::mismatchingBitAndError(const Token *tok1, const MathLib::bi reportError(locations, Severity::style, "mismatchingBitAnd", - msg.str()); + msg.str(), CWE398, false); } @@ -317,7 +322,7 @@ void CheckCondition::comparisonError(const Token *tok, const std::string &bitop, "spot sometimes. In case of complex expression it might help to split it to " "separate expressions."); - reportError(tok, Severity::style, "comparisonError", errmsg); + reportError(tok, Severity::style, "comparisonError", errmsg, CWE398, false); } bool CheckCondition::isOverlappingCond(const Token * const cond1, const Token * const cond2, const std::set &constFunctions) const @@ -395,7 +400,7 @@ void CheckCondition::multiConditionError(const Token *tok, unsigned int line1) errmsg << "Expression is always false because 'else if' condition matches previous condition at line " << line1 << "."; - reportError(tok, Severity::style, "multiCondition", errmsg.str()); + reportError(tok, Severity::style, "multiCondition", errmsg.str(), CWE398, false); } //--------------------------------------------------------------------------- @@ -481,7 +486,7 @@ void CheckCondition::oppositeInnerConditionError(const Token *tok1, const Token* std::list callstack; callstack.push_back(tok1); callstack.push_back(tok2); - reportError(callstack, Severity::warning, "oppositeInnerCondition", "Opposite conditions in nested 'if' blocks lead to a dead code block."); + reportError(callstack, Severity::warning, "oppositeInnerCondition", "Opposite conditions in nested 'if' blocks lead to a dead code block.", CWE398, false); } //--------------------------------------------------------------------------- @@ -805,17 +810,17 @@ void CheckCondition::incorrectLogicOperatorError(const Token *tok, const std::st reportError(tok, Severity::warning, "incorrectLogicOperator", "Logical disjunction always evaluates to true: " + condition + ".\n" "Logical disjunction always evaluates to true: " + condition + ". " - "Are these conditions necessary? Did you intend to use && instead? Are the numbers correct? Are you comparing the correct variables?"); + "Are these conditions necessary? Did you intend to use && instead? Are the numbers correct? Are you comparing the correct variables?", CWE571, false); else reportError(tok, Severity::warning, "incorrectLogicOperator", "Logical conjunction always evaluates to false: " + condition + ".\n" "Logical conjunction always evaluates to false: " + condition + ". " - "Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?"); + "Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?", CWE570, false); } void CheckCondition::redundantConditionError(const Token *tok, const std::string &text) { - reportError(tok, Severity::style, "redundantCondition", "Redundant condition: " + text); + reportError(tok, Severity::style, "redundantCondition", "Redundant condition: " + text, CWE398, false); } //----------------------------------------------------------------------------- @@ -854,7 +859,7 @@ void CheckCondition::checkModuloAlwaysTrueFalse() void CheckCondition::moduloAlwaysTrueFalseError(const Token* tok, const std::string& maxVal) { reportError(tok, Severity::warning, "moduloAlwaysTrueFalse", - "Comparison of modulo result is predetermined, because it is always less than " + maxVal + "."); + "Comparison of modulo result is predetermined, because it is always less than " + maxVal + ".", CWE398, false); } //--------------------------------------------------------------------------- @@ -955,7 +960,7 @@ void CheckCondition::clarifyConditionError(const Token *tok, bool assign, bool b reportError(tok, Severity::style, "clarifyCondition", - errmsg); + errmsg, CWE398, false); } diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 9e79246cc..d976e85de 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -40,6 +40,9 @@ static const char ExtraVersion[] = ""; static TimerResults S_timerResults; +// CWE ids used +static const CWE CWE398(398U); // Indicator of Poor Code Quality + CppCheck::CppCheck(ErrorLogger &errorLogger, bool useGlobalSuppressions) : _errorLogger(errorLogger), exitcode(0), _useGlobalSuppressions(useGlobalSuppressions), tooManyConfigs(false), _simplify(true) { @@ -523,7 +526,7 @@ void CppCheck::tooManyConfigsError(const std::string &file, const std::size_t nu ErrorLogger::ErrorMessage errmsg(loclist, Severity::information, msg.str(), - "toomanyconfigs", + "toomanyconfigs", CWE398, false); reportErr(errmsg); diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index e4cf4af92..876e9097e 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -58,6 +58,19 @@ ErrorLogger::ErrorMessage::ErrorMessage(const std::list &callStack setmsg(msg); } + + +ErrorLogger::ErrorMessage::ErrorMessage(const std::list &callStack, Severity::SeverityType severity, const std::string &msg, const std::string &id, const CWE &cwe, bool inconclusive) : + _callStack(callStack), // locations for this error message + _id(id), // set the message id + _severity(severity), // severity for this error message + _cwe(cwe.id), + _inconclusive(inconclusive) +{ + // set the summary and verbose messages + setmsg(msg); +} + ErrorLogger::ErrorMessage::ErrorMessage(const std::list& callstack, const TokenList* list, Severity::SeverityType severity, const std::string& id, const std::string& msg, bool inconclusive) : _id(id), _severity(severity), _cwe(0U), _inconclusive(inconclusive) { @@ -76,6 +89,25 @@ ErrorLogger::ErrorMessage::ErrorMessage(const std::list& callstack setmsg(msg); } + +ErrorLogger::ErrorMessage::ErrorMessage(const std::list& callstack, const TokenList* list, Severity::SeverityType severity, const std::string& id, const std::string& msg, const CWE &cwe, bool inconclusive) + : _id(id), _severity(severity), _cwe(cwe.id), _inconclusive(inconclusive) +{ + // Format callstack + for (std::list::const_iterator it = callstack.begin(); it != callstack.end(); ++it) { + // --errorlist can provide null values here + if (!(*it)) + continue; + + _callStack.push_back(ErrorLogger::ErrorMessage::FileLocation(*it, list)); + } + + if (list && !list->getFiles().empty()) + file0 = list->getFiles()[0]; + + setmsg(msg); +} + void ErrorLogger::ErrorMessage::setmsg(const std::string &msg) { // If a message ends to a '\n' and contains only a one '\n' diff --git a/lib/errorlogger.h b/lib/errorlogger.h index 2402770ed..bbb956d8d 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -27,6 +27,13 @@ #include #include +struct CWE { + explicit CWE(unsigned short ID) : id(ID) {} + unsigned short id; +}; + + + class Token; class TokenList; @@ -198,7 +205,9 @@ public: }; ErrorMessage(const std::list &callStack, Severity::SeverityType severity, const std::string &msg, const std::string &id, bool inconclusive); + ErrorMessage(const std::list &callStack, Severity::SeverityType severity, const std::string &msg, const std::string &id, const CWE &cwe, bool inconclusive); ErrorMessage(const std::list& callstack, const TokenList* list, Severity::SeverityType severity, const std::string& id, const std::string& msg, bool inconclusive); + ErrorMessage(const std::list& callstack, const TokenList* list, Severity::SeverityType severity, const std::string& id, const std::string& msg, const CWE &cwe, bool inconclusive); ErrorMessage(); /**