From 729f57d8f1d3a4541cf9ae462b06094455ee5fb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 9 Mar 2019 22:14:02 +0100 Subject: [PATCH] Start a major rewrite of CheckBufferOverrun. For now only the 'array index' and 'buffer overflow' checks are rewritten. There are important TODOs still; for instance adding CTU support using our CTU infrastructure, add handling of pointers (maybe I'll use FwdAnalysis for this), add handling of multidimensional arrays, etc.. --- lib/checkbufferoverrun.cpp | 2228 ++++-------------------------------- lib/checkbufferoverrun.h | 241 +--- lib/library.cpp | 18 +- lib/library.h | 2 +- test/testbufferoverrun.cpp | 418 +++---- 5 files changed, 435 insertions(+), 2472 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 715587372..2186e1e7b 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -60,1570 +60,39 @@ static const CWE CWE788(788U); // Access of Memory Location After End of Buffer //--------------------------------------------------------------------------- -static void makeArrayIndexOutOfBoundsError(std::ostream& oss, const CheckBufferOverrun::ArrayInfo &arrayInfo, const std::vector &index) -{ - oss << "$symbol:" << arrayInfo.varname() << '\n'; - oss << "Array '$symbol"; - for (std::size_t i = 0; i < arrayInfo.num().size(); ++i) - oss << "[" << arrayInfo.num(i) << "]"; - if (index.size() == 1) - oss << "' accessed at index " << index[0] << ", which is"; - else { - oss << "' index " << arrayInfo.varname(); - for (std::size_t i = 0; i < index.size(); ++i) - oss << "[" << index[i] << "]"; - } - oss << " out of bounds."; -} -void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector &index) -{ - std::ostringstream oss; - makeArrayIndexOutOfBoundsError(oss, arrayInfo, index); - reportError(tok, Severity::error, "arrayIndexOutOfBounds", oss.str(), CWE788, false); -} - -void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector &index) -{ - bool inconclusive = false; - const Token *condition = nullptr; - for (std::size_t i = 0; i < index.size(); ++i) { - inconclusive |= index[i].isInconclusive(); - if (condition == nullptr) - condition = index[i].condition; - } - - std::list errorPath; - if (mSettings->xml || !mSettings->templateLocation.empty()) { - for (std::size_t i = 0; i < index.size(); ++i) { - const ErrorPath &e = getErrorPath(tok, &index[i], ""); - for (ErrorPath::const_iterator it = e.begin(); it != e.end(); ++it) { - const std::string &info = it->second; - if (info.empty()) - continue; - std::string nr; - if (index.size() > 1U) - nr = "(" + MathLib::toString(i + 1) + getOrdinalText(i + 1) + " array index) "; - errorPath.emplace_back(it->first, nr + info); - } - } - errorPath.emplace_back(tok,"Array index out of bounds"); - } else { - errorPath.emplace_back(tok, "Array index out of bounds"); - if (condition) - errorPath.emplace_back(condition, "Assuming that condition '" + condition->expressionString() + "' is not redundant"); - } - - if (condition != nullptr) { - if (!mSettings->isEnabled(Settings::WARNING)) - return; - - std::ostringstream errmsg; - errmsg << "$symbol:" << arrayInfo.varname() << '\n'; - errmsg << ValueFlow::eitherTheConditionIsRedundant(condition) << " or the array '$symbol"; - for (std::size_t i = 0; i < arrayInfo.num().size(); ++i) - errmsg << "[" << arrayInfo.num(i) << "]"; - if (index.size() == 1) - errmsg << "' is accessed at index " << index[0].intvalue << ", which is out of bounds."; - else { - errmsg << "' index " << arrayInfo.varname(); - for (std::size_t i = 0; i < index.size(); ++i) - errmsg << "[" << index[i].intvalue << "]"; - errmsg << " is out of bounds."; - } - - reportError(errorPath, Severity::warning, "arrayIndexOutOfBoundsCond", errmsg.str(), CWE119, inconclusive); - } else { - std::ostringstream errmsg; - errmsg << "$symbol:" << arrayInfo.varname() << '\n'; - errmsg << "Array '$symbol"; - for (std::size_t i = 0; i < arrayInfo.num().size(); ++i) - errmsg << "[" << arrayInfo.num(i) << "]"; - if (index.size() == 1) - errmsg << "' accessed at index " << index[0].intvalue << ", which is out of bounds."; - else { - errmsg << "' index " << arrayInfo.varname(); - for (std::size_t i = 0; i < index.size(); ++i) - errmsg << "[" << index[i].intvalue << "]"; - errmsg << " out of bounds."; - } - - reportError(errorPath, Severity::error, "arrayIndexOutOfBounds", errmsg.str(), CWE119, inconclusive); - } -} - -void CheckBufferOverrun::arrayIndexOutOfBoundsError(const std::list &callstack, const ArrayInfo &arrayInfo, const std::vector &index) -{ - std::ostringstream oss; - makeArrayIndexOutOfBoundsError(oss, arrayInfo, index); - reportError(callstack, Severity::error, "arrayIndexOutOfBounds", oss.str()); -} - -static std::string bufferOverrunMessage(std::string varnames) -{ - varnames.erase(std::remove(varnames.begin(), varnames.end(), ' '), varnames.end()); - - std::string errmsg("Buffer is accessed out of bounds"); - if (!varnames.empty()) - errmsg = "$symbol:" + varnames + '\n' + errmsg + ": " + varnames; - else - errmsg += "."; - - return errmsg; -} - -void CheckBufferOverrun::bufferOverrunError(const Token *tok, const std::string &varnames) -{ - reportError(tok, Severity::error, "bufferAccessOutOfBounds", bufferOverrunMessage(varnames), CWE788, false); -} - - -void CheckBufferOverrun::bufferOverrunError(const std::list &callstack, const std::string &varnames) -{ - reportError(callstack, Severity::error, "bufferAccessOutOfBounds", bufferOverrunMessage(varnames)); -} - -void CheckBufferOverrun::possibleBufferOverrunError(const Token *tok, const std::string &src, const std::string &dst, bool cat) -{ - if (cat) - 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.", 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.", CWE398, false); -} - -void CheckBufferOverrun::strncatUsageError(const Token *tok) -{ - if (mSettings && !mSettings->isEnabled(Settings::WARNING)) - return; - - reportError(tok, Severity::warning, "strncatUsage", - "Dangerous usage of strncat - 3rd parameter is the maximum number of characters to append.\n" - "At most, strncat appends the 3rd parameter's amount of characters and adds a terminating null byte.\n" - "The safe way to use strncat is to subtract one from the remaining space in the buffer and use it as 3rd parameter." - "Source: http://www.cplusplus.com/reference/cstring/strncat/\n" - "Source: http://www.opensource.apple.com/source/Libc/Libc-167/gen.subproj/i386.subproj/strncat.c", CWE119, false); -} - -void CheckBufferOverrun::outOfBoundsError(const Token *tok, const std::string &what, const bool show_size_info, const MathLib::bigint &supplied_size, const MathLib::bigint &actual_size) -{ - std::ostringstream oss; - - oss << what << " is out of bounds"; - if (show_size_info) - oss << ": Supplied size " << supplied_size << " is larger than actual size " << actual_size; - oss << '.'; - reportError(tok, Severity::error, "outOfBounds", oss.str(), CWE788, false); -} - -void CheckBufferOverrun::pointerOutOfBoundsError(const Token *tok, const Token *index, const MathLib::bigint indexvalue) -{ - // The severity is portability instead of error since this ub doesn't - // cause bad behaviour on most implementations. people create out - // of bounds pointers by intention. - const std::string expr(tok ? tok->expressionString() : std::string()); - std::string errmsg; - if (index && !index->isNumber()) { - errmsg = "Undefined behaviour, when '" + - index->expressionString() + - "' is " + - MathLib::toString(indexvalue) + - " the pointer arithmetic '" + expr + "' is out of bounds"; - } else { - errmsg = "Undefined behaviour, pointer arithmetic '" + expr + "' is out of bounds"; - } - const 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, CWE398, false); - /* - "Undefined behaviour: The result of this pointer arithmetic does not point into - or just one element past the end of the " + object + ". - Further information: - https://www.securecoding.cert.org/confluence/display/seccode/ARR30-C.+Do+not+form+or+use+out+of+bounds+pointers+or+array+subscripts"); - */ -} - -void CheckBufferOverrun::sizeArgumentAsCharError(const Token *tok) -{ - if (mSettings && !mSettings->isEnabled(Settings::WARNING)) - return; - reportError(tok, Severity::warning, "sizeArgumentAsChar", "The size argument is given as a char constant.", CWE682, false); -} - - -void CheckBufferOverrun::terminateStrncpyError(const Token *tok, const std::string &varname) -{ - const std::string shortMessage = "The buffer '$symbol' may not be null-terminated after the call to strncpy()."; - reportError(tok, Severity::warning, "terminateStrncpy", - "$symbol:" + varname + '\n' + - shortMessage + '\n' + - shortMessage + ' ' + - "If the source string's size fits or exceeds the given size, strncpy() does not add a " - "zero at the end of the buffer. This causes bugs later in the code if the code " - "assumes buffer is null-terminated.", CWE170, true); -} - -void CheckBufferOverrun::cmdLineArgsError(const Token *tok) -{ - reportError(tok, Severity::error, "insecureCmdLineArgs", "Buffer overrun possible for long command line arguments.", CWE119, false); -} - -void CheckBufferOverrun::bufferNotZeroTerminatedError(const Token *tok, const std::string &varname, const std::string &function) -{ - const std::string errmsg = "$symbol:" + varname + '\n' + - "$symbol:" + function + '\n' + - "The buffer '" + varname + "' is not null-terminated after the call to " + function + "().\n" - "The buffer '" + varname + "' is not null-terminated after the call to " + function + "(). " - "This will cause bugs later in the code if the code assumes the buffer is null-terminated."; - - reportError(tok, Severity::warning, "bufferNotZeroTerminated", errmsg, CWE170, true); -} - -void CheckBufferOverrun::argumentSizeError(const Token *tok, const std::string &functionName, const std::string &varname) -{ - reportError(tok, Severity::warning, "argumentSize", - "$symbol:" + functionName + '\n' + - "$symbol:" + varname + '\n' + - "The array '" + varname + "' is too small, the function '" + functionName + "' expects a bigger one.", CWE398, false); -} - -void CheckBufferOverrun::negativeMemoryAllocationSizeError(const Token *tok) -{ - reportError(tok, Severity::error, "negativeMemoryAllocationSize", - "Memory allocation size is negative.\n" - "Memory allocation size is negative." - "Negative allocation size has no specified behaviour.", CWE131, false); -} - -//--------------------------------------------------------------------------- - - -//--------------------------------------------------------------------------- -// Check array usage.. -//--------------------------------------------------------------------------- - - -static bool isAddressOf(const Token *tok) -{ - const Token *tok2 = tok->astParent(); - while (Token::Match(tok2, "%name%|.|::|[")) - tok2 = tok2->astParent(); - return tok2 && tok2->isUnaryOp("&"); -} - -/** - * bailout if variable is used inside if/else/switch block or if there is "break" - * @param tok token for "if" or "switch" - * @param varid variable id - * @return is bailout recommended? - */ -static bool bailoutIfSwitch(const Token *tok, const unsigned int varid) -{ - const Token* end = tok->linkAt(1)->linkAt(1); - if (Token::simpleMatch(end, "} else {")) // scan the else-block - end = end->linkAt(2); - if (Token::simpleMatch(end, "{")) // Ticket #5203: Invalid code, bailout - return true; - - // Used later to check if the body belongs to a "if" - const bool is_if = tok->str() == "if"; - - for (; tok && tok != end; tok = tok->next()) { - // If scanning a "if" block then bailout for "break" - if (is_if && (tok->str() == "break" || tok->str() == "continue")) - return true; - - // bailout for "return" - else if (tok->str() == "return") - return true; - - // bailout if varid is found - else if (tok->varId() == varid) - return true; - } - - // No bailout stuff found => return false - return false; -} -//--------------------------------------------------------------------------- - -static bool checkMinSizes(const std::vector &minsizes, const Token * const ftok, const MathLib::bigint arraySize, const Token **charSizeToken, const Settings * const settings) -{ - if (charSizeToken) - *charSizeToken = nullptr; - - if (minsizes.empty()) - return false; - - // All conditions must be true - bool error = true; - for (std::vector::const_iterator minsize = minsizes.begin(); minsize != minsizes.end(); ++minsize) { - if (!error) - return false; - error = false; - const Token *argtok = ftok->tokAt(2); - for (int argnum = 1; argtok && argnum < minsize->arg; argnum++) - argtok = argtok->nextArgument(); - if (!argtok) - return false; - switch (minsize->type) { - case Library::ArgumentChecks::MinSize::ARGVALUE: - if (Token::Match(argtok, "%num% ,|)")) { - const MathLib::bigint sz = MathLib::toLongNumber(argtok->str()); - if (sz > arraySize) - error = true; - } else if (argtok->tokType() == Token::eChar && Token::Match(argtok->next(), ",|)") && charSizeToken) - *charSizeToken = argtok; //sizeArgumentAsCharError(argtok); - break; - case Library::ArgumentChecks::MinSize::MUL: - // TODO: handle arbitrary arg2 - if (minsize->arg2 == minsize->arg+1 && Token::Match(argtok, "%num% , %num% ,|)")) { - const MathLib::bigint sz = MathLib::toLongNumber(argtok->str()) * MathLib::toLongNumber(argtok->strAt(2)); - if (sz > arraySize) - error = true; - } - break; - case Library::ArgumentChecks::MinSize::STRLEN: { - if (settings->library.isargformatstr(ftok,minsize->arg)) { - std::list parameters; - const std::string &formatstr(argtok->str()); - const Token *argtok2 = argtok; - while ((argtok2 = argtok2->nextArgument()) != nullptr) { - if (Token::Match(argtok2, "%num%|%str% [,)]")) - parameters.push_back(argtok2); - else - parameters.push_back(nullptr); - } - const MathLib::biguint len = CheckBufferOverrun::countSprintfLength(formatstr, parameters); - if (len > arraySize + 2) - error = true; - } else { - const Token *strtoken = argtok->getValueTokenMaxStrLength(); - if (strtoken && Token::getStrLength(strtoken) >= arraySize) - error = true; - } - } - break; - case Library::ArgumentChecks::MinSize::SIZEOF: - if (argtok->tokType() == Token::eString && Token::getStrLength(argtok) >= arraySize) - error = true; - break; - case Library::ArgumentChecks::MinSize::NONE: - return false; - }; - } - return error; -} - -void CheckBufferOverrun::checkFunctionParameter(const Token &ftok, unsigned int paramIndex, const ArrayInfo &arrayInfo, const std::list& callstack) -{ - const std::vector * const minsizes = mSettings->library.argminsizes(&ftok, paramIndex); - - if (minsizes) { - MathLib::bigint arraySize = arrayInfo.element_size(); - if (arraySize == 0) - return; - for (std::size_t i = 0; i < arrayInfo.num().size(); ++i) - arraySize *= arrayInfo.num(i); - - // dimension is 0 or unknown => bailout - if (arraySize == 0) - return; - - const Token *charSizeToken = nullptr; - if (checkMinSizes(*minsizes, &ftok, arraySize, &charSizeToken, mSettings)) - bufferOverrunError(callstack, arrayInfo.varname()); - if (charSizeToken) - sizeArgumentAsCharError(charSizeToken); - } - - // Calling a user function? - // only 1-dimensional arrays can be checked currently - else if (arrayInfo.num().size() == 1) { - const Function* const func = ftok.function(); - - if (func && func->hasBody()) { - // Get corresponding parameter.. - const Variable* const parameter = func->getArgumentVar(paramIndex-1); - - // Ensure that it has a compatible size.. - if (!parameter || sizeOfType(parameter->typeStartToken()) != arrayInfo.element_size()) - return; - - // No variable id occur for instance when: - // - Variable function arguments: "void f(...)" - // - Unnamed parameter: "void f(char *)" - if (parameter->declarationId() == 0) - return; - - // Check the parameter usage in the function scope.. - for (const Token* ftok2 = func->functionScope->bodyStart; ftok2 != func->functionScope->bodyEnd; ftok2 = ftok2->next()) { - if (Token::Match(ftok2, "if|for|switch|while (")) { - // bailout if there is buffer usage.. - if (bailoutIfSwitch(ftok2, parameter->declarationId())) { - break; - } - - // no bailout is needed. skip the if-block - else { - // goto end of if block.. - ftok2 = ftok2->linkAt(1)->linkAt(1); - if (Token::simpleMatch(ftok2, "} else {")) - ftok2 = ftok2->linkAt(2); - if (!ftok2) - break; - continue; - } - } - - if (ftok2->str() == "}") - break; - - if (ftok2->varId() == parameter->declarationId()) { - if (Token::Match(ftok2->previous(), "-- %name%") || - Token::Match(ftok2, "%name% --")) - break; - - if (Token::Match(ftok2->previous(), ";|{|}|%op% %name% [ %num% ]")) { - const MathLib::bigint index = MathLib::toLongNumber(ftok2->strAt(2)); - if (index >= 0 && arrayInfo.num(0) > 0 && index >= arrayInfo.num(0)) { - std::list callstack2(callstack); - callstack2.push_back(ftok2); - - const std::vector indexes(1, index); - arrayIndexOutOfBoundsError(callstack2, arrayInfo, indexes); - } - } - } - - // Calling function.. - if (Token::Match(ftok2, "%name% (")) { - ArrayInfo ai(arrayInfo); - ai.declarationId(parameter->declarationId()); - checkFunctionCall(ftok2, ai, callstack); - } - } - } - } - - // Check 'float x[10]' arguments in declaration - if (mSettings->isEnabled(Settings::WARNING)) { - const Function* const func = ftok.function(); - - // If argument is '%type% a[num]' then check bounds against num - if (func) { - const Variable* const argument = func->getArgumentVar(paramIndex-1); - const Token *nameToken; - if (argument && Token::Match(argument->typeStartToken(), "%type% %var% [ %num% ] [,)[]") - && (nameToken = argument->nameToken()) != nullptr) { - const Token *tok2 = nameToken->next(); - - MathLib::bigint argsize = sizeOfType(argument->typeStartToken()); - if (argsize == 100) // unknown size - argsize = 0; - do { - argsize *= MathLib::toLongNumber(tok2->strAt(1)); - tok2 = tok2->tokAt(3); - } while (Token::Match(tok2, "[ %num% ] [,)[]")); - - MathLib::bigint arraysize = arrayInfo.element_size(); - if (arraysize == 100) // unknown size - arraysize = 0; - for (std::size_t i = 0; i < arrayInfo.num().size(); i++) - arraysize *= arrayInfo.num(i); - - if (Token::Match(tok2, "[,)]") && arraysize > 0 && argsize > arraysize) - argumentSizeError(&ftok, ftok.str(), arrayInfo.varname()); - } - } - } -} - - -void CheckBufferOverrun::checkFunctionCall(const Token *tok, const ArrayInfo &arrayInfo, std::list callstack) -{ - // Don't go deeper than 2 levels, the checking can get very slow - // when there is no limit - if (callstack.size() >= 2) - return; - - // Prevent recursion - for (std::list::const_iterator it = callstack.cbegin(); it != callstack.cend(); ++it) { - // Same function name => bail out - if (tok->str() == (*it)->str()) - return; - } - callstack.push_back(tok); - - const unsigned int declarationId = arrayInfo.declarationId(); - - const Token *argtok = tok->tokAt(2); - unsigned int argnr = 1U; - while (argtok) { - if (Token::Match(argtok, "%varid% ,|)", declarationId)) - checkFunctionParameter(*tok, argnr, arrayInfo, callstack); - else if (Token::Match(argtok, "%varid% + %num% ,|)", declarationId)) { - const ArrayInfo ai(arrayInfo.limit(MathLib::toLongNumber(argtok->strAt(2)))); - checkFunctionParameter(*tok, argnr, ai, callstack); - } - // goto next parameter.. - argtok = argtok->nextArgument(); - argnr++; - } -} - -void CheckBufferOverrun::checkScope(const Token *tok, const std::vector &varname, const ArrayInfo &arrayInfo) -{ - const MathLib::bigint size = arrayInfo.num(0); - if (size <= 0) // unknown size - return; - - if (tok->str() == "return") { - tok = tok->next(); - if (!tok) - return; - } - - const bool printInconclusive = mSettings->inconclusive; - const MathLib::bigint total_size = arrayInfo.element_size() * size; - const unsigned int declarationId = arrayInfo.declarationId(); - - std::string varnames; - for (std::size_t i = 0; i < varname.size(); ++i) - varnames += (i == 0 ? "" : " . ") + *varname[i]; - - const int varcount = varname.empty() ? 0 : static_cast((varname.size() - 1) * 2U); - - // ValueFlow array index.. - if ((declarationId > 0 && Token::Match(tok, "%varid% [", declarationId)) || - (declarationId == 0 && Token::simpleMatch(tok, (varnames + " [").c_str()))) { - - const Token *tok2 = tok->next(); - while (tok2->str() != "[") - tok2 = tok2->next(); - valueFlowCheckArrayIndex(tok2, arrayInfo); - } - - // If the result of pointer arithmetic means that the pointer is - // out of bounds then this flag will be set. - bool pointerIsOutOfBounds = false; - - const bool printPortability = mSettings->isEnabled(Settings::PORTABILITY); - - for (const Token* const end = tok->scope()->bodyEnd; tok && tok != end; tok = tok->next()) { - if (declarationId != 0 && Token::Match(tok, "%varid% = new|malloc|realloc", declarationId)) { - // Abort - break; - } - - // reassign buffer - if (declarationId > 0 && Token::Match(tok, "[;{}] %varid% = %any%", declarationId)) { - // using varid .. bailout - if (tok->tokAt(3)->varId() != declarationId) - break; - pointerIsOutOfBounds = false; - } - - // Array index.. - if ((declarationId > 0 && ((tok->str() == "return" || (!tok->isName() && !Token::Match(tok, "[.&]"))) && Token::Match(tok->next(), "%varid% [", declarationId))) || - (declarationId == 0 && ((tok->str() == "return" || (!tok->isName() && !Token::Match(tok, "[.&]"))) && (Token::Match(tok->next(), (varnames + " [").c_str()) || Token::Match(tok->next(), (*varname[0] +" [ %num% ] . " + *varname[1] + " [ %num% ]").c_str()))))) { - std::vector indexes; - const Token *tok2 = tok->tokAt(2 + varcount); - for (; Token::Match(tok2, "[ %num% ]"); tok2 = tok2->tokAt(3)) { - const MathLib::bigint index = MathLib::toLongNumber(tok2->strAt(1)); - indexes.push_back(index); - } - for (; Token::Match(tok2->tokAt(3), "[ %num% ]"); tok2 = tok2->tokAt(3)) { - const MathLib::bigint index = MathLib::toLongNumber(tok2->strAt(4)); - indexes.push_back(index); - } - if (indexes.empty() && arrayInfo.num().size() == 1U && Token::simpleMatch(tok2, "[") && tok2->astOperand2()) { - const ValueFlow::Value *value = tok2->astOperand2()->getMaxValue(false); - if (value) { - indexes.push_back(value->intvalue); - } - } - - if (indexes.size() == arrayInfo.num().size()) { - // Check if the indexes point outside the whole array.. - // char a[10][10]; - // a[0][20] <-- ok. - // a[9][20] <-- error. - - // total number of elements of array.. - MathLib::bigint totalElements = 1; - - // total index.. - MathLib::bigint totalIndex = 0; - - // calculate the totalElements and totalIndex.. - for (std::size_t i = 0; i < indexes.size(); ++i) { - const std::size_t ri = indexes.size() - 1 - i; - totalIndex += indexes[ri] * totalElements; - totalElements *= arrayInfo.num(ri); - if (arrayInfo.num(ri) == -1) { - // unknown size - totalElements = 0; - break; - } - } - - // totalElements == 0 => Unknown size - if (totalElements == 0) - continue; - - const Token *tok3 = tok->previous(); - while (tok3 && Token::Match(tok3->previous(), "%name% .")) - tok3 = tok3->tokAt(-2); - - // taking address of 1 past end? - if (totalIndex == totalElements) { - const bool addr = (tok3 && (tok3->str() == "&" || - Token::simpleMatch(tok3->previous(), "& ("))); - if (addr) - continue; - } - - // Is totalIndex in bounds? - if (totalIndex > totalElements || totalIndex < 0) { - arrayIndexOutOfBoundsError(tok->tokAt(1 + varcount), arrayInfo, indexes); - } - // Is any array index out of bounds? - else { - // check each index for overflow - for (std::size_t i = 0; i < indexes.size(); ++i) { - if (indexes[i] >= arrayInfo.num(i)) { - if (indexes.size() == 1U) { - arrayIndexOutOfBoundsError(tok->tokAt(1 + varcount), arrayInfo, indexes); - break; // only warn about the first one - } - - // The access is still within the memory range for the array - // so it may be intentional. - else if (printInconclusive) { - arrayIndexOutOfBoundsError(tok->tokAt(1 + varcount), arrayInfo, indexes); - break; // only warn about the first one - } - } - } - } - } - tok = tok2; - continue; - } - - // memset, memcmp, memcpy, strncpy, fgets.. - if (declarationId == 0 && Token::Match(tok, "%name% ( !!)")) { - std::list callstack(1, tok); - const Token* tok2 = tok->tokAt(2); - if (Token::Match(tok2, (varnames + " ,").c_str())) - checkFunctionParameter(*tok, 1, arrayInfo, callstack); - tok2 = tok2->nextArgument(); - if (Token::Match(tok2, (varnames + " ,").c_str())) - checkFunctionParameter(*tok, 2, arrayInfo, callstack); - } - - if (total_size > 0) { - // Writing data into array.. - if ((declarationId > 0 && Token::Match(tok, "strcpy|strcat ( %varid% , %str%|%var% )", declarationId)) || - (declarationId == 0 && Token::Match(tok, ("strcpy|strcat ( " + varnames + " , %str%|%var% )").c_str()))) { - const Token* lastParamTok = tok->tokAt(varcount + 4); - if (lastParamTok->tokType() == Token::Type::eString) { - const std::size_t len = Token::getStrLength(lastParamTok); - if (len >= total_size) { - bufferOverrunError(tok, declarationId > 0 ? emptyString : varnames); - continue; - } - } else { - const Variable *var = lastParamTok->variable(); - if (var && var->isArray() && var->dimensions().size() == 1) { - const MathLib::bigint len = var->dimension(0); - if (len > total_size) { - if (printInconclusive) - possibleBufferOverrunError(tok, tok->strAt(4), tok->strAt(2), tok->str() == "strcat"); - continue; - } - } - } - } - - // Detect few strcat() calls - const std::string strcatPattern = declarationId > 0 ? std::string("strcat ( %varid% , %str% ) ;") : ("strcat ( " + varnames + " , %str% ) ;"); - if (Token::Match(tok, strcatPattern.c_str(), declarationId)) { - std::size_t charactersAppend = 0; - const Token *tok2 = tok; - - do { - charactersAppend += Token::getStrLength(tok2->tokAt(4 + varcount)); - if (charactersAppend >= static_cast(total_size)) { - bufferOverrunError(tok2); - break; - } - tok2 = tok2->tokAt(7 + varcount); - } while (Token::Match(tok2, strcatPattern.c_str(), declarationId)); - } - - // Check function call.. - if (Token::Match(tok, "%name% (")) { - // No varid => function calls are not handled - if (declarationId == 0) - continue; - - const ArrayInfo arrayInfo1(declarationId, varnames, total_size / size, size); - const std::list callstack; - checkFunctionCall(tok, arrayInfo1, callstack); - } - } - - // undefined behaviour: result of pointer arithmetic is out of bounds - if (declarationId && Token::Match(tok, "= %varid% + %num% ;", declarationId)) { - const MathLib::bigint index = MathLib::toLongNumber(tok->strAt(3)); - if (printPortability && index > size) - pointerOutOfBoundsError(tok->tokAt(2)); - if (index >= size && Token::Match(tok->tokAt(-2), "[;{}] %varid% =", declarationId)) - pointerIsOutOfBounds = true; - } - - else if (pointerIsOutOfBounds && Token::Match(tok, "[;{}=] * %varid% [;=]", declarationId)) { - outOfBoundsError(tok->tokAt(2), tok->strAt(2), false, 0, 0); - } - } -} - -static std::vector valueFlowGetArrayIndexes(const Token * const tok, bool conditional, std::size_t dimensions) -{ - unsigned int indexvarid = 0; - const std::vector empty; - std::vector indexes; - for (const Token *tok2 = tok; indexes.size() < dimensions && Token::simpleMatch(tok2, "["); tok2 = tok2->link()->next()) { - if (!tok2->astOperand2()) - return empty; - - const ValueFlow::Value *index = tok2->astOperand2()->getMaxValue(conditional); - if (!index) - return empty; - if (indexvarid == 0U) - indexvarid = index->varId; - if (index->varId > 0 && indexvarid != index->varId) - return empty; - if (index->intvalue < 0) - return empty; - indexes.push_back(*index); - } - - return indexes; -} - - -void CheckBufferOverrun::valueFlowCheckArrayIndex(const Token * const tok, const ArrayInfo &arrayInfo) -{ - // Declaration in global scope or namespace? - if (tok->scope()->type == Scope::eGlobal || tok->scope()->type == Scope::eNamespace) - return; - /* - { - const Token *parent = tok->astParent(); - while (Token::Match(parent, "%name%|::|*|&")) - parent = parent->astParent(); - if (parent && !Token::simpleMatch(parent, "=")) - return; - } - */ - const bool printInconclusive = mSettings->inconclusive; - // Taking address? - const bool addressOf = isAddressOf(tok); - - // Look for errors first - for (int warn = 0; warn == 0 || warn == 1; ++warn) { - // Negative index.. - for (const Token *tok2 = tok; tok2 && tok2->str() == "["; tok2 = tok2->link()->next()) { - const Token *index = tok2->astOperand2(); - if (!index) - continue; - const ValueFlow::Value *value = index->getValueLE(-1LL,mSettings); - if (value) - negativeIndexError(index, *value); - } - - // Index out of bounds.. - const std::vector indexes(valueFlowGetArrayIndexes(tok, warn==1, arrayInfo.num().size())); - if (indexes.size() != arrayInfo.num().size()) - continue; - - // Check if the indexes point outside the whole array.. - // char a[10][10]; - // a[0][20] <-- ok. - // a[9][20] <-- error. - - // total number of elements of array.. - const MathLib::bigint totalElements = arrayInfo.numberOfElements(); - - // total index.. - const MathLib::bigint totalIndex = arrayInfo.totalIndex(indexes); - - // totalElements <= 0 => Unknown size - if (totalElements <= 0) - continue; - - if (addressOf && totalIndex == totalElements) - continue; - - // Is totalIndex in bounds? - if (totalIndex >= totalElements) { - arrayIndexOutOfBoundsError(tok, arrayInfo, indexes); - break; - } - - // Is any array index out of bounds? - if (printInconclusive) { - // check each index for overflow - for (std::size_t i = 0; i < indexes.size(); ++i) { - if (indexes[i].intvalue >= arrayInfo.num(i)) { - // The access is still within the memory range for the array - // so it may be intentional. - arrayIndexOutOfBoundsError(tok, arrayInfo, indexes); - break; // only warn about the first one - } - } - } - } -} - - - -void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo) -{ - bool reassigned = false; - - for (const Token* const end = tok->scope()->bodyEnd; tok != end; tok = tok->next()) { - if (reassigned && tok->str() == ";") - break; - - if (tok->varId() != arrayInfo.declarationId()) - continue; - - if (tok->strAt(1) == "=") { - reassigned = true; - } - - checkScope_inner(tok, arrayInfo); - } -} - -void CheckBufferOverrun::checkScope(const Token *tok, std::map arrayInfos) -{ - unsigned int reassigned = 0; - - for (const Token* const end = tok->scope()->bodyEnd; tok != end; tok = tok->next()) { - if (reassigned && tok->str() == ";") { - arrayInfos.erase(reassigned); - reassigned = 0; - } - - if (!tok->variable() || tok->variable()->nameToken() == tok) - continue; - - const std::map::const_iterator arrayInfo = arrayInfos.find(tok->varId()); - if (arrayInfo == arrayInfos.cend()) - continue; - - if (tok->strAt(1) == "=") { - reassigned = tok->varId(); - } - - checkScope_inner(tok, arrayInfo->second); - } -} - -void CheckBufferOverrun::checkScope_inner(const Token *tok, const ArrayInfo &arrayInfo) -{ - const bool printPortability = mSettings->isEnabled(Settings::PORTABILITY); - const bool printWarning = mSettings->isEnabled(Settings::WARNING); - const bool printInconclusive = mSettings->inconclusive; - - const Token *astParent = tok->astParent(); - - if (tok->strAt(1) == "[") { - valueFlowCheckArrayIndex(tok->next(), arrayInfo); - } - - else if (printPortability && !tok->isCast() && astParent && astParent->str() == "+") { - // undefined behaviour: result of pointer arithmetic is out of bounds - const Token *index; - if (tok == astParent->astOperand1()) - index = astParent->astOperand2(); - else - index = astParent->astOperand1(); - if (index) { - const ValueFlow::Value *value = index->getValueGE(arrayInfo.num(0) + 1U, mSettings); - if (!value) - value = index->getValueLE(-1, mSettings); - if (value) - pointerOutOfBoundsError(astParent, index, value->intvalue); - } - } - - else if (printPortability && astParent && astParent->str() == "-") { - const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); - const Variable *var = symbolDatabase->getVariableFromVarId(arrayInfo.declarationId()); - if (var && var->isArray()) { - const Token *index = astParent->astOperand2(); - const ValueFlow::Value *value = index ? index->getValueGE(1,mSettings) : nullptr; - if (index && !value) - value = index->getValueLE(-1 - arrayInfo.num(0), mSettings); - if (value) - pointerOutOfBoundsError(astParent, index, value->intvalue); - } - } - - if (!tok->scope()->isExecutable()) // No executable code outside of executable scope - continue to increase performance - return; - - const Token* tok2 = astParent; - if (tok2) { - while (tok2->astParent() && !Token::Match(tok2->astParent(), "[,(]")) - tok2 = tok2->astParent(); - while (tok2->astParent() && tok2->astParent()->str() == ",") - tok2 = tok2->astParent(); - if (tok2->astParent() && tok2->astParent()->str() == "(") - tok2 = tok2->astParent(); - - if (tok2->str() != "(") - return; - - tok2 = tok2->previous(); - - // Check function call.. - checkFunctionCall(tok2, arrayInfo, std::list()); - - const MathLib::biguint total_size = arrayInfo.num(0) * arrayInfo.element_size(); - - if (printWarning && printInconclusive && Token::Match(tok2, "strncpy|memcpy|memmove ( %varid% , %str% , %num% )", arrayInfo.declarationId())) { - if (Token::getStrLength(tok2->tokAt(4)) >= total_size) { - const MathLib::biguint num = MathLib::toULongNumber(tok2->strAt(6)); - if (total_size == num) - bufferNotZeroTerminatedError(tok2, tok2->strAt(2), tok2->str()); - } - } - - if (printWarning && Token::Match(tok2, "strncpy|strncat ( %varid% ,", arrayInfo.declarationId())) { - const std::vector args = getArguments(tok2); - const Token * const sizeArg = args.size() == 3 ? args[2] : nullptr; - const bool knownSize = sizeArg && sizeArg->hasKnownIntValue(); - const Token * const endToken = tok2->linkAt(1); - const bool smallerSrcString = (args.size() == 3 && - args[1]->hasKnownValue() && - args[1]->values().front().isTokValue() && - args[1]->values().front().tokvalue->tokType() == Token::eString && - knownSize && - Token::getStrLength(args[1]->values().front().tokvalue) < sizeArg->getKnownIntValue()); - - // check for strncpy which is not terminated - if (knownSize && !smallerSrcString && tok2->str() == "strncpy") { - // strncpy takes entire variable length as input size - const MathLib::bigint num = sizeArg->getKnownIntValue(); - - // this is currently 'inconclusive'. See TestBufferOverrun::terminateStrncpy3 - if (printInconclusive && num >= total_size) { - const Token *tok4 = tok2->next()->link()->next(); - for (; tok4; tok4 = tok4->next()) { - const Token* tok3 = tok2->tokAt(2); - if (tok4->varId() == tok3->varId()) { - const Token *eq = nullptr; - if (Token::Match(tok4, "%varid% [", tok3->varId()) && Token::simpleMatch(tok4->linkAt(1), "] =")) - eq = tok4->linkAt(1)->next(); - const Token *rhs = eq ? eq->astOperand2() : nullptr; - if (!(rhs && rhs->hasKnownIntValue() && rhs->getValue(0))) - terminateStrncpyError(tok2, tok3->str()); - break; - } - } - } - } - - // Dangerous usage of strncat.. - else if (knownSize && !smallerSrcString && tok2->str() == "strncat") { - const MathLib::bigint n = sizeArg->getKnownIntValue(); - if (n >= total_size) - strncatUsageError(tok2); - } - - // Dangerous usage of strncpy + strncat.. - if (knownSize && Token::Match(endToken, ") ; strncat ( %varid% ,", arrayInfo.declarationId())) { - const std::vector args2 = getArguments(endToken->tokAt(2)); - const Token *sizeArg2 = args2.size() == 3 ? args2[2] : nullptr; - if (sizeArg2 && sizeArg2->hasKnownIntValue()) { - const MathLib::bigint n = sizeArg->getKnownIntValue() + sizeArg2->getKnownIntValue(); - if (n > total_size) - strncatUsageError(endToken->tokAt(4)); - } - } - } - - // Writing data into array.. - if (total_size > 0) { - if (Token::Match(tok2, "strcpy ( %varid% , %str% )", arrayInfo.declarationId())) { - const std::size_t len = Token::getStrLength(tok2->tokAt(4)); - if (len >= total_size) { - bufferOverrunError(tok2, arrayInfo.varname()); - return; - } - } - - // Detect few strcat() calls - MathLib::biguint charactersAppend = 0; - const Token *tok3 = tok2; - - while (Token::Match(tok3, "strcat ( %varid% , %str% )", arrayInfo.declarationId())) { - charactersAppend += Token::getStrLength(tok3->tokAt(4)); - if (charactersAppend >= total_size) { - bufferOverrunError(tok3, arrayInfo.varname()); - break; - } - tok3 = tok3->tokAt(7); - } - } - } -} - -//--------------------------------------------------------------------------- -// Negative size in array declarations -//--------------------------------------------------------------------------- - -static bool isVLAIndex(const Token *tok) -{ - if (!tok) - return false; - if (tok->varId() != 0U) - return true; - if (tok->str() == "?") { - // this is a VLA index if both expressions around the ":" is VLA index - if (tok->astOperand2() && - tok->astOperand2()->str() == ":" && - isVLAIndex(tok->astOperand2()->astOperand1()) && - isVLAIndex(tok->astOperand2()->astOperand2())) - return true; - return false; - } - return isVLAIndex(tok->astOperand1()) || isVLAIndex(tok->astOperand2()); -} - -void CheckBufferOverrun::negativeArraySize() -{ - const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); - for (const Variable *var : symbolDatabase->variableList()) { - if (!var || !var->isArray()) - continue; - const Token * const nameToken = var->nameToken(); - if (!Token::Match(nameToken, "%var% [") || !nameToken->next()->astOperand2()) - continue; - const ValueFlow::Value *sz = nameToken->next()->astOperand2()->getValueLE(-1,mSettings); - // don't warn about constant negative index because that is a compiler error - if (sz && isVLAIndex(nameToken->next()->astOperand2())) - negativeArraySizeError(nameToken); - } -} - -void CheckBufferOverrun::negativeArraySizeError(const Token *tok) -{ - const std::string arrayName = tok ? tok->expressionString() : std::string(); - const std::string line1 = arrayName.empty() ? std::string() : ("$symbol:" + arrayName + '\n'); - reportError(tok, Severity::error, "negativeArraySize", - line1 + - "Declaration of array '" + arrayName + "' with negative size is undefined behaviour", CWE758, false); -} - -//--------------------------------------------------------------------------- -// Checking member variables of structs. -//--------------------------------------------------------------------------- -bool CheckBufferOverrun::isArrayOfStruct(const Token* tok, int &position) -{ - if (Token::Match(tok->next(), "%name% [ %num% ]")) { - tok = tok->tokAt(4); - int i = 1; - for (;;) { - if (Token::Match(tok->next(), "[ %num% ]")) { - i++; - tok = tok->tokAt(4); - } else - break; - } - if (Token::simpleMatch(tok->next(),";")) { - position = i; - return true; - } - } - return false; -} - -//--------------------------------------------------------------------------- -// Checking local variables in a scope -//--------------------------------------------------------------------------- - -void CheckBufferOverrun::checkGlobalAndLocalVariable() -{ - const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); - - // check string literals - for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) { - if (Token::Match(tok, "%str% [") && tok->next()->astOperand2()) { - const std::size_t size = Token::getStrSize(tok); - const ValueFlow::Value *value = tok->next()->astOperand2()->getMaxValue(false); - if (value && value->intvalue >= (isAddressOf(tok) ? size + 1U : size)) - bufferOverrunError(tok, tok->str()); - } - - if (Token::Match(tok, "%var% [") && tok->next()->astOperand2() && tok->variable() && tok->variable()->isPointer()) { - const ValueFlow::Value *value = tok->next()->astOperand2()->getMaxValue(false); - if (!value) - continue; - - for (std::list::const_iterator it = tok->values().begin(); it != tok->values().end(); ++it) { - if (!it->isTokValue() || !it->tokvalue) - continue; - const Variable *var = it->tokvalue->variable(); - if (var && var->isArray()) { - if (astCanonicalType(tok) != astCanonicalType(it->tokvalue)) - continue; - - const ArrayInfo arrayInfo(var, symbolDatabase); - const MathLib::bigint elements = arrayInfo.numberOfElements(); - if (elements <= 0) // unknown size - continue; - - const std::vector indexes(valueFlowGetArrayIndexes(tok->next(), false, var->dimensions().size())); - if (indexes.size() != var->dimensions().size()) - continue; - - const MathLib::bigint index = arrayInfo.totalIndex(indexes); - if (index < (isAddressOf(tok) ? elements + 1U : elements)) - continue; - - std::list callstack = { it->tokvalue, tok }; - - std::vector indexes2(indexes.size()); - for (unsigned int i = 0; i < indexes.size(); ++i) - indexes2[i] = indexes[i].intvalue; - - arrayIndexOutOfBoundsError(callstack, arrayInfo, indexes2); - } - } - } - } - - // check all known fixed size arrays first by just looking them up - for (const Scope &scope : symbolDatabase->scopeList) { - std::map arrayInfos; - for (const Variable &var : scope.varlist) { - if (!var.isArray() || var.dimension(0) <= 0) - continue; - mErrorLogger->reportProgress(mTokenizer->list.getSourceFilePath(), - "Check (BufferOverrun::checkGlobalAndLocalVariable 1)", - var.nameToken()->progressValue()); - - if (mTokenizer->isMaxTime()) - return; - - const Token *tok = var.nameToken(); - do { - if (tok->str() == "{") { - if (Token::simpleMatch(tok->previous(), "= {")) - tok = tok->link(); - else - break; - } - tok = tok->next(); - } while (tok && tok->str() != ";"); - if (!tok) - break; - arrayInfos[var.declarationId()] = ArrayInfo(&var, symbolDatabase, var.declarationId()); - } - if (!arrayInfos.empty()) - checkScope(scope.bodyStart ? scope.bodyStart : mTokenizer->tokens(), arrayInfos); - } - - const std::vector v; - - // find all dynamically allocated arrays next - for (const Scope * scope : symbolDatabase->functionScopes) { - - for (const Token *tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) { - if (!Token::Match(tok, "[*;{}] %var% =")) - continue; - - // size : Max array index - MathLib::bigint size = 0; - - // nextTok : used to skip to next statement. - const Token * nextTok = tok; - - mErrorLogger->reportProgress(mTokenizer->list.getSourceFilePath(), - "Check (BufferOverrun::checkGlobalAndLocalVariable 2)", - tok->progressValue()); - - if (mTokenizer->isMaxTime()) - return; - - // varid : The variable id for the array - const Variable *var = tok->next()->variable(); - // FIXME: This is an ugly fix for a crash. The SymbolDatabase - // should create the variable. - if (!var) - continue; - - if (mTokenizer->isCPP() && Token::Match(tok, "[*;{}] %var% = new %type% [")) { - tok = tok->tokAt(5); - if (tok->astOperand2() == nullptr || tok->astOperand2()->getMaxValue(false) == nullptr) - continue; - size = tok->astOperand2()->getMaxValue(false)->intvalue; - nextTok = tok->link()->next(); - if (size < 0) { - negativeMemoryAllocationSizeError(tok); - } - } else if (mTokenizer->isCPP() && Token::Match(tok, "[*;{}] %var% = new %type% (|;")) { - size = 1; - tok = tok->tokAt(5); - if (tok->str() == ";") - nextTok = tok->next(); - else - nextTok = tok->link()->next(); - } else if (Token::Match(tok, "[*;{}] %var% = malloc|alloca (") && Token::simpleMatch(tok->linkAt(4), ") ;")) { - tok = tok->tokAt(4); - if (tok->astOperand2() == nullptr || tok->astOperand2()->getMaxValue(false) == nullptr) - continue; - size = tok->astOperand2()->getMaxValue(false)->intvalue; - nextTok = tok->link()->tokAt(2); - - if (size < 0) { - negativeMemoryAllocationSizeError(tok); - } - - /** @todo false negatives: this may be too conservative */ - if (!var->isPointer() || var->typeStartToken()->next() != var->typeEndToken()) - continue; - - // malloc() gets count of bytes and not count of - // elements, so we should calculate count of elements - // manually - const unsigned int typeSize = sizeOfType(var->typeStartToken()); - if (typeSize > 0) { - size /= static_cast(typeSize); - } - if (size < 0) { - negativeMemoryAllocationSizeError(tok); - } - } else { - continue; - } - - if (var == nullptr) - continue; - - const MathLib::bigint totalSize = size * static_cast(sizeOfType(var->typeStartToken())); - if (totalSize == 0) - continue; - - const ArrayInfo temp(var->declarationId(), var->name(), totalSize / size, size); - checkScope(nextTok, v, temp); - } - } -} -//--------------------------------------------------------------------------- - - -//--------------------------------------------------------------------------- -// Checking member variables of structs. -//--------------------------------------------------------------------------- - -void CheckBufferOverrun::checkStructVariable() -{ - // find every class and struct - const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); - for (const Scope * scope : symbolDatabase->classAndStructScopes) { - for (const Variable &var : scope->varlist) { - if (!var.isArray()) - continue; - // create ArrayInfo from the array variable - ArrayInfo arrayInfo(&var, symbolDatabase); - - // find every function - for (const Scope * func_scope : symbolDatabase->functionScopes) { - // If struct is declared in a function then check - // if scope_func matches - if (scope->nestedIn->type == Scope::eFunction && - scope->nestedIn != func_scope) { - continue; - } - - // check for member variables - if (func_scope->functionOf == scope) { - // only check non-empty function - if (func_scope->bodyStart->next() != func_scope->bodyEnd) { - // start checking after the { - const Token *tok = func_scope->bodyStart->next(); - checkScope(tok, arrayInfo); - } - } - - // skip inner scopes.. - /** @todo false negatives: handle inner scopes someday */ - if (scope->nestedIn->isClassOrStruct()) - continue; - - std::vector varname = { nullptr, &arrayInfo.varname() }; - - // search the function and it's parameters - for (const Token *tok3 = func_scope->classDef; tok3 && tok3 != func_scope->bodyEnd; tok3 = tok3->next()) { - // search for the class/struct name - if (tok3->str() != scope->className) - continue; - - // find all array variables - int posOfSemicolon = -1; - - // Declare variable: Fred fred1; - if (Token::Match(tok3->next(), "%var% ;")) - varname[0] = &tok3->strAt(1); - - else if (isArrayOfStruct(tok3,posOfSemicolon)) - varname[0] = &tok3->strAt(1); - - // Declare pointer or reference: Fred *fred1 - else if (Token::Match(tok3->next(), "*|& %var% [,);=]")) - varname[0] = &tok3->strAt(2); - - else - continue; - - // check for variable sized structure - if (scope->type == Scope::eStruct && var.isPublic()) { - // last member of a struct with array size of 0 or 1 could be a variable sized structure - if (var.dimensions().size() == 1 && var.dimension(0) < 2 && - var.index() == (scope->varlist.size() - 1)) { - // dynamically allocated so could be variable sized structure - if (tok3->next()->str() == "*") { - // check for allocation - if ((Token::Match(tok3->tokAt(3), "; %name% = malloc ( %num% ) ;") || - (Token::Match(tok3->tokAt(3), "; %name% = (") && - Token::Match(tok3->linkAt(6), ") malloc ( %num% ) ;"))) && - (tok3->strAt(4) == tok3->strAt(2))) { - MathLib::bigint size; - - // find size of allocation - if (tok3->strAt(3) == "(") // has cast - size = MathLib::toLongNumber(tok3->linkAt(6)->strAt(3)); - else - size = MathLib::toLongNumber(tok3->strAt(8)); - - // We don't calculate the size of a structure even when we know - // the size of the members. We just assign a length of 100 for - // any struct. If the size is less than 100, we assume the - // programmer knew the size and specified it rather than using - // sizeof(struct). If the size is greater than 100, we assume - // the programmer specified the size as sizeof(struct) + number. - // Either way, this is just a guess and could be wrong. The - // information to make the right decision has been simplified - // away by the time we get here. - if (size != 100) { // magic number for size of struct - // check if a real size was specified and give up - // malloc(10) rather than malloc(sizeof(struct)) - if (size < 100 || arrayInfo.element_size() == 0) - continue; - - // calculate real array size based on allocated size - const MathLib::bigint elements = (size - 100) / arrayInfo.element_size(); - arrayInfo.num(0, arrayInfo.num(0) + elements); - } - } - - // size unknown so assume it is a variable sized structure - else - continue; - } - } - } - - // Goto end of statement. - const Token *checkTok = nullptr; - while (tok3 && tok3 != func_scope->bodyEnd) { - // End of statement. - if (tok3->str() == ";") { - checkTok = tok3; - break; - } - - // End of function declaration.. - if (Token::simpleMatch(tok3, ") ;")) - break; - - // Function implementation.. - if (Token::simpleMatch(tok3, ") {")) { - checkTok = tok3->tokAt(2); - break; - } - - tok3 = tok3->next(); - } - - if (!tok3) - break; - - if (!checkTok) - continue; - - // Check variable usage.. - ArrayInfo temp = arrayInfo; - temp.declarationId(0); // do variable lookup by variable and member names rather than varid - std::string varnames; // use class and member name for messages - for (std::size_t k = 0; k < varname.size(); ++k) - varnames += (k == 0 ? "" : ".") + *varname[k]; - - temp.varname(varnames); - checkScope(checkTok, varname, temp); - } - } - } - } -} - -//--------------------------------------------------------------------------- - -void CheckBufferOverrun::bufferOverrun() -{ - const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); - - // singlepass checking using ast, symboldatabase and valueflow - for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) { - if (mSettings->isEnabled(Settings::PORTABILITY) && tok->str() == "+" && tok->valueType() && tok->valueType()->pointer > 0) { - if (!tok->astOperand1() || !tok->astOperand1()->valueType()) - continue; - if (!tok->astOperand2() || !tok->astOperand2()->valueType()) - continue; - - // pointer arithmetic.. - const Token *pointerToken, *indexToken; - - if (tok->astOperand1()->valueType()->pointer == 0) { - indexToken = tok->astOperand1(); - pointerToken = tok->astOperand2(); - } else if (tok->astOperand2()->valueType()->pointer == 0) { - indexToken = tok->astOperand2(); - pointerToken = tok->astOperand1(); - } else { - continue; - } - - while (pointerToken && pointerToken->str() == ".") - pointerToken = pointerToken->astOperand2(); - - if (!pointerToken || !pointerToken->isName()) - continue; - - const Variable *var = pointerToken->variable(); - if (!var || !var->isArray() || var->dimension(0) <= 0) - continue; - - const ValueFlow::Value *value = indexToken->getValueGE(var->dimension(0)+1, mSettings); - if (value) { - pointerOutOfBoundsError(tok, indexToken, value->intvalue); - } - } - - // Array index - if (!Token::Match(tok, "%name% [")) - continue; - - // TODO: what to do about negative index.. - const Token *index = tok->next()->astOperand2(); - if (index && index->getValueLE(-1LL,mSettings)) - continue; - - // Set full varname.. - std::string varname; - if (tok->astParent() && tok->astParent()->str() == ".") { - const Token *parent = tok->astParent(); - while (parent->astParent() && parent->astParent()->str() == ".") - parent = parent->astParent(); - varname = parent->expressionString(); - } else - varname = tok->str(); - - - const Variable * const var = tok->variable(); - if (!var) - continue; - - const Token * const strtoken = tok->getValueTokenMinStrSize(); - if (strtoken && !var->isArray()) { - // TODO: check for access to symbol inside the array bounds, but outside the stored string: - // char arr[10] = "123"; - // arr[7] = 'x'; // warning: arr[7] is inside the array bounds, but past the string's end - - if (tok->valueType() && tok->valueType()->type == ValueType::Type::CONTAINER) - continue; - - const ArrayInfo arrayInfo(tok->varId(), varname, 1U, Token::getStrSize(strtoken)); - valueFlowCheckArrayIndex(tok->next(), arrayInfo); - } else { - if (var->nameToken() == tok || !var->isArray()) - continue; - - // unknown array dimensions - bool known = true; - for (unsigned int i = 0; i < var->dimensions().size(); ++i) { - known &= (var->dimension(i) >= 1); - known &= var->dimensionKnown(i); - } - if (!known) - continue; - - // TODO: last array in struct.. - if (var->dimension(0) <= 1 && Token::simpleMatch(var->nameToken()->linkAt(1),"] ; }")) - continue; - - if (var->scope() && var->scope()->type == Scope::eUnion) - continue; - - ArrayInfo arrayInfo(var, symbolDatabase); - arrayInfo.varname(varname); - - valueFlowCheckArrayIndex(tok->next(), arrayInfo); - } - } -} -//--------------------------------------------------------------------------- - -MathLib::biguint CheckBufferOverrun::countSprintfLength(const std::string &input_string, const std::list ¶meters) +static size_t getMinFormatStringOutputLength(const std::vector ¶meters, unsigned int formatStringArgNr) { + if (formatStringArgNr == 0 || formatStringArgNr > parameters.size()) + return 0; + if (parameters[formatStringArgNr - 1]->tokType() != Token::eString) + return 0; + const std::string &formatString = parameters[formatStringArgNr - 1]->str(); bool percentCharFound = false; - std::size_t input_string_size = 1; + std::size_t outputStringSize = 0; bool handleNextParameter = false; std::string digits_string; bool i_d_x_f_found = false; - std::list::const_iterator paramIter = parameters.begin(); std::size_t parameterLength = 0; - for (std::string::size_type i = 0; i < input_string.length(); ++i) { - if (input_string[i] == '\\') { - if (i < input_string.length() - 1 && input_string[i + 1] == '0') + unsigned int inputArgNr = formatStringArgNr; + for (std::string::size_type i = 1; i + 1 < formatString.length(); ++i) { + if (formatString[i] == '\\') { + if (i < formatString.length() - 1 && formatString[i + 1] == '0') break; - ++input_string_size; + ++outputStringSize; ++i; continue; } if (percentCharFound) { - switch (input_string[i]) { + switch (formatString[i]) { case 'f': case 'x': case 'X': case 'i': i_d_x_f_found = true; handleNextParameter = true; + parameterLength = 1; // TODO break; case 'c': case 'e': @@ -1634,31 +103,34 @@ MathLib::biguint CheckBufferOverrun::countSprintfLength(const std::string &input case 'p': case 'n': handleNextParameter = true; + parameterLength = 1; // TODO break; case 'd': i_d_x_f_found = true; - if (paramIter != parameters.end() && *paramIter && (*paramIter)->tokType() != Token::eString) - parameterLength = (*paramIter)->str().length(); + parameterLength = 1; + if (inputArgNr < parameters.size() && parameters[inputArgNr]->hasKnownIntValue()) + parameterLength = MathLib::toString(parameters[inputArgNr]->getKnownIntValue()).length(); handleNextParameter = true; break; case 's': - if (paramIter != parameters.end() && *paramIter && (*paramIter)->tokType() == Token::eString) - parameterLength = Token::getStrLength(*paramIter); + parameterLength = 0; + if (inputArgNr < parameters.size() && parameters[inputArgNr]->tokType() == Token::eString) + parameterLength = Token::getStrLength(parameters[inputArgNr]); handleNextParameter = true; break; } } - if (input_string[i] == '%') + if (formatString[i] == '%') percentCharFound = !percentCharFound; else if (percentCharFound) { - digits_string.append(1, input_string[i]); + digits_string.append(1, formatString[i]); } if (!percentCharFound) - input_string_size++; + outputStringSize++; if (handleNextParameter) { unsigned int tempDigits = static_cast(std::abs(std::atoi(digits_string.c_str()))); @@ -1669,7 +141,7 @@ MathLib::biguint CheckBufferOverrun::countSprintfLength(const std::string &input const std::string endStr = digits_string.substr(digits_string.find('.') + 1); const unsigned int maxLen = std::max(static_cast(std::abs(std::atoi(endStr.c_str()))), 1U); - if (input_string[i] == 's') { + if (formatString[i] == 's') { // For strings, the length after the dot "%.2s" will limit // the length of the string. if (parameterLength > maxLen) @@ -1683,472 +155,244 @@ MathLib::biguint CheckBufferOverrun::countSprintfLength(const std::string &input } if (tempDigits < parameterLength) - input_string_size += parameterLength; + outputStringSize += parameterLength; else - input_string_size += tempDigits; + outputStringSize += tempDigits; parameterLength = 0; digits_string.clear(); i_d_x_f_found = false; percentCharFound = false; handleNextParameter = false; - if (paramIter != parameters.end()) - ++paramIter; + ++inputArgNr; } } - return input_string_size; + return outputStringSize; +} + +//--------------------------------------------------------------------------- + +void CheckBufferOverrun::arrayIndex() +{ + for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) { + if (!Token::Match(tok, "%name% [") || !tok->variable() || tok->variable()->nameToken() == tok) + continue; + if (!tok->scope()->isExecutable()) { + // LHS in non-executable scope => This is just a definition + const Token *parent = tok->next(); + while (parent && !Token::simpleMatch(parent->astParent(), "=")) + parent = parent->astParent(); + if (parent && parent == parent->astParent()->astOperand1()) + continue; + } + const Token *indexToken = tok->next()->astOperand2(); + if (!indexToken) + continue; + + const Token *stringLiteral = nullptr; + + if (tok->variable()->dimensions().empty()) { + stringLiteral = tok->getValueTokenMinStrSize(); + if (!stringLiteral) + continue; + } + + const MathLib::bigint dim = stringLiteral ? Token::getStrSize(stringLiteral) : tok->variable()->dimensions()[0].num; + + // Positive index + if (stringLiteral || dim > 1) { // TODO check arrays with dim 1 also + for (int cond = 0; cond < 2; cond++) { + const ValueFlow::Value *value = indexToken->getMaxValue(cond == 1); + if (!value) + continue; + const MathLib::bigint index = value->intvalue; + if (index < dim) + continue; + if (index == dim) { + const Token *parent = tok->next(); + while (Token::simpleMatch(parent, "[")) + parent = parent->astParent(); + if (parent->str() == "&") + continue; + } + arrayIndexError(tok->next(), tok->variable(), value); + } + } + + // Negative index + const ValueFlow::Value *negativeValue = indexToken->getValueLE(-1, mSettings); + if (negativeValue) { + negativeIndexError(tok->next(), tok->variable(), negativeValue); + } + } +} + +static std::string arrayIndexMessage(const Token *tok, const Variable *var, const ValueFlow::Value *index) { + std::string array = tok->astOperand1()->expressionString(); + for (const Dimension &dim : var->dimensions()) + array += "[" + MathLib::toString(dim.num) + "]"; + + std::ostringstream errmsg; + if (index->condition) + errmsg << ValueFlow::eitherTheConditionIsRedundant(index->condition) + << " or the array '" + array + "' is accessed at index " << index->intvalue << ", which is out of bounds."; + else + errmsg << "Array '" << array << "' accessed at index " << index->intvalue << ", which is out of bounds."; + + return errmsg.str(); +} + +void CheckBufferOverrun::arrayIndexError(const Token *tok, const Variable *var, const ValueFlow::Value *index) +{ + if (!tok) { + reportError(tok, Severity::error, "arrayIndexOutOfBounds", "Array 'arr[16]' accessed at index 16, which is out of bounds.", CWE788, false); + return; + } + + reportError(getErrorPath(tok, index, "Array index out of bounds"), + index->errorSeverity() ? Severity::error : Severity::warning, + "arrayIndexOutOfBounds", + arrayIndexMessage(tok, var, index), + CWE788, + index->isInconclusive()); +} + +void CheckBufferOverrun::negativeIndexError(const Token *tok, const Variable *var, const ValueFlow::Value *negativeValue) +{ + if (!negativeValue) { + reportError(tok, Severity::error, "negativeIndex", "Negative array index", CWE786, false); + return; + } + + if (!negativeValue->errorSeverity() && !mSettings->isEnabled(Settings::WARNING)) + return; + + reportError(getErrorPath(tok, negativeValue, "Negative array index"), + negativeValue->errorSeverity() ? Severity::error : Severity::warning, + "negativeIndex", + arrayIndexMessage(tok, var, negativeValue), + CWE786, + negativeValue->isInconclusive()); } +size_t CheckBufferOverrun::getBufferSize(const Token *bufTok) const +{ + if (!bufTok->valueType()) + return 0; + const Variable *var = bufTok->variable(); + if (!var) + return 0; + if (!var->dimensions().empty()) { + MathLib::bigint dim = 1; + for (const Dimension &d : var->dimensions()) + dim *= d.num; + switch (bufTok->valueType()->type) { + case ValueType::Type::BOOL: + return dim * mSettings->sizeof_bool; + case ValueType::Type::CHAR: + return dim; + case ValueType::Type::SHORT: + return dim * mSettings->sizeof_short; + case ValueType::Type::INT: + return dim * mSettings->sizeof_int; + case ValueType::Type::LONG: + return dim * mSettings->sizeof_long; + case ValueType::Type::LONGLONG: + return dim * mSettings->sizeof_long_long; + case ValueType::Type::FLOAT: + return dim * mSettings->sizeof_float; + case ValueType::Type::DOUBLE: + return dim * mSettings->sizeof_double; + case ValueType::Type::LONGDOUBLE: + return dim * mSettings->sizeof_long_double; + default: + // TODO: Get size of other types + break; + }; + return 0; + } + // TODO: For pointers get pointer value.. + return 0; +} -//--------------------------------------------------------------------------- -// Checking for allocating insufficient memory for copying a string by -// allocating only strlen(src) bytes instead of strlen(src) + 1 bytes (one -// extra for the terminating null character). -// Example: -// char *b = malloc(strlen(a)); // Should be malloc(strlen(a) + 1); -// strcpy(b, a); // <== Buffer overrun -//--------------------------------------------------------------------------- -void CheckBufferOverrun::checkBufferAllocatedWithStrlen() +static bool checkBufferSize(const Token *ftok, const Library::ArgumentChecks::MinSize &minsize, const std::vector &args, const MathLib::bigint bufferSize, const Settings *settings) +{ + const Token * const arg = (minsize.arg > 0 && minsize.arg - 1 < args.size()) ? args[minsize.arg - 1] : nullptr; + const Token * const arg2 = (minsize.arg2 > 0 && minsize.arg2 - 1 < args.size()) ? args[minsize.arg2 - 1] : nullptr; + + switch (minsize.type) { + case Library::ArgumentChecks::MinSize::Type::STRLEN: + if (settings->library.isargformatstr(ftok, minsize.arg)) { + return getMinFormatStringOutputLength(args, minsize.arg) < bufferSize; + } else if (arg) { + const Token *strtoken = arg->getValueTokenMaxStrLength(); + if (strtoken) + return Token::getStrLength(strtoken) < bufferSize; + } + break; + case Library::ArgumentChecks::MinSize::Type::ARGVALUE: + if (arg && arg->hasKnownIntValue()) + return arg->getKnownIntValue() <= bufferSize; + break; + case Library::ArgumentChecks::MinSize::Type::SIZEOF: + // TODO + break; + case Library::ArgumentChecks::MinSize::Type::MUL: + if (arg && arg2 && arg->hasKnownIntValue() && arg2->hasKnownIntValue()) + return (arg->getKnownIntValue() * arg2->getKnownIntValue()) <= bufferSize; + break; + case Library::ArgumentChecks::MinSize::Type::NONE: + break; + }; + return true; +} + + +void CheckBufferOverrun::bufferOverflow() { const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); for (const Scope * scope : symbolDatabase->functionScopes) { - for (const Token *tok = scope->bodyStart->next(); tok && tok != scope->bodyEnd; tok = tok->next()) { - const unsigned int dstVarId = tok->varId(); - if (!dstVarId || tok->strAt(1) != "=") - continue; - - tok = tok->tokAt(2); - - unsigned int srcVarId; - // Look for allocation of a buffer based on the size of a string - if (Token::Match(tok, "malloc|g_malloc|g_try_malloc|alloca ( strlen ( %var% ) )")) { - const Token *varTok = tok->tokAt(4); - srcVarId = varTok->varId(); - tok = varTok->tokAt(2); - } else if (mTokenizer->isCPP() && Token::Match(tok, "new char [ strlen ( %var% ) ]")) { - const Token *varTok = tok->tokAt(5); - srcVarId = varTok->varId(); - tok = varTok->tokAt(2); - } else if (Token::Match(tok, "realloc|g_realloc|g_try_realloc ( %name% , strlen ( %var% ) )")) { - const Token *varTok = tok->tokAt(6); - srcVarId = varTok->varId(); - tok = varTok->tokAt(2); - } else - continue; - - // To avoid false positives and added complexity, we will only look for - // improper usage of the buffer within the block that it was allocated - for (const Token* const end = tok->scope()->bodyEnd; tok && tok->next() && tok != end; tok = tok->next()) { - // If the buffers are modified, we can't be sure of their sizes - if (tok->varId() == srcVarId || tok->varId() == dstVarId) - break; - - if (Token::Match(tok, "strcpy ( %varid% , %var% )", dstVarId) && - tok->tokAt(4)->varId() == srcVarId) { - bufferOverrunError(tok); - } - } - if (!tok) - return; - } - } -} - -//--------------------------------------------------------------------------- -// memcpy(temp, "hello world", 50); -//--------------------------------------------------------------------------- -void CheckBufferOverrun::checkStringArgument() -{ - const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); - for (const Scope * const scope : symbolDatabase->functionScopes) { for (const Token *tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) { - if (!Token::Match(tok, "%name% (") || !mSettings->library.hasminsize(tok->str())) + if (!Token::Match(tok, "%name% (") || Token::simpleMatch(tok, ") {")) continue; - - unsigned int argnr = 1; - for (const Token *argtok = tok->tokAt(2); argtok; argtok = argtok->nextArgument(), argnr++) { - if (!Token::Match(argtok, "%str% ,|)")) + if (!mSettings->library.hasminsize(tok)) + continue; + const std::vector args = getArguments(tok); + for (unsigned int argnr = 0; argnr < args.size(); ++argnr) { + if (!args[argnr]->valueType() || args[argnr]->valueType()->pointer == 0) continue; - const Token *strtoken = argtok->getValueTokenMinStrSize(); - if (!strtoken) + const std::vector *minsizes = mSettings->library.argminsizes(tok, argnr + 1); + if (!minsizes || minsizes->empty()) continue; - const std::vector *minsizes = mSettings->library.argminsizes(tok, argnr); - if (!minsizes) + // Get buffer size.. + const Token *argtok = args[argnr]; + while (argtok && argtok->isCast()) + argtok = argtok->astOperand2() ? argtok->astOperand2() : argtok->astOperand1(); + while (Token::Match(argtok, ".|::")) + argtok = argtok->astOperand2(); + if (!argtok || !argtok->variable()) continue; - if (checkMinSizes(*minsizes, tok, Token::getStrSize(strtoken), nullptr, mSettings)) - bufferOverrunError(argtok); - } - } - } -} - -//--------------------------------------------------------------------------- -// 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 SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); - const std::size_t functions = symbolDatabase->functionScopes.size(); - for (std::size_t i = 0; i < functions; ++i) { - const Function * function = symbolDatabase->functionScopes[i]->function; - if (function) { - const Token* tok = function->token; - - // Get the name of the argv variable - unsigned int argvVarid = 0; - if (Token::simpleMatch(tok, "main (")) - tok = tok->tokAt(2); - else - continue; - - 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; - - // Jump to the opening curly brace - tok = symbolDatabase->functionScopes[i]->bodyStart; - - // 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() == argvVarid) - break; - - // Update varid in case the input is copied by strdup() - if (Token::Match(tok->tokAt(-2), "%var% = strdup ( %varid%", argvVarid)) - argvVarid = tok->tokAt(-2)->varId(); - - // Match common patterns that can result in a buffer overrun - // e.g. strcpy(buffer, argv[0]) - if (Token::Match(tok, "strcpy|strcat (")) { - const Token *nextArgument = tok->tokAt(2)->nextArgument(); - if (nextArgument) - tok = nextArgument; - else - continue; // Ticket #7964 - if (Token::Match(tok, "* %varid%", argvVarid) || // e.g. strcpy(buf, * ptr) - Token::Match(tok, "%varid% [", argvVarid) || // e.g. strcpy(buf, argv[1]) - Token::Match(tok, "%varid%", argvVarid)) // e.g. strcpy(buf, pointer) - cmdLineArgsError(tok); + // TODO: strcpy(buf+10, "hello"); + const size_t bufferSize = getBufferSize(argtok); + if (bufferSize <= 1) + continue; + bool error = true; + for (const Library::ArgumentChecks::MinSize &minsize : *minsizes) { + if (checkBufferSize(tok, minsize, args, bufferSize, mSettings)) { + error = false; + break; + } } - } - } - } -} -//--------------------------------------------------------------------------- - - -void CheckBufferOverrun::negativeIndexError(const Token *tok, MathLib::bigint index) -{ - std::ostringstream ostr; - ostr << "Array index " << index << " is out of bounds."; - reportError(tok, Severity::error, "negativeIndex", ostr.str(), CWE786, false); -} - -void CheckBufferOverrun::negativeIndexError(const Token *tok, const ValueFlow::Value &index) -{ - const ErrorPath errorPath = getErrorPath(tok, &index, "Negative array index"); - std::ostringstream errmsg; - if (index.condition) - errmsg << ValueFlow::eitherTheConditionIsRedundant(index.condition) - << ", otherwise there is negative array index " << index.intvalue << "."; - else - errmsg << "Array index " << index.intvalue << " is out of bounds."; - reportError(errorPath, index.errorSeverity() ? Severity::error : Severity::warning, "negativeIndex", errmsg.str(), CWE786, index.isInconclusive()); -} - -CheckBufferOverrun::ArrayInfo::ArrayInfo() - : mElementSize(0), mDeclarationId(0) -{ -} - -CheckBufferOverrun::ArrayInfo::ArrayInfo(const Variable *var, const SymbolDatabase * symbolDatabase, const unsigned int forcedeclid) - : mVarName(var->name()), mDeclarationId((forcedeclid == 0U) ? var->declarationId() : forcedeclid) -{ - for (std::size_t i = 0; i < var->dimensions().size(); i++) - mNum.push_back(var->dimension(i)); - if (var->typeEndToken()->str() == "*") - mElementSize = symbolDatabase->sizeOfType(var->typeEndToken()); - else if (var->typeStartToken()->strAt(-1) == "struct") - mElementSize = 100; - else { - mElementSize = symbolDatabase->sizeOfType(var->typeEndToken()); - } -} - -/** - * Create array info with specified data - * The intention is that this is only a temporary solution.. all - * checking should be based on ArrayInfo from the start and then - * this will not be needed as the declare can be used instead. - */ -CheckBufferOverrun::ArrayInfo::ArrayInfo(unsigned int id, const std::string &name, MathLib::bigint size1, MathLib::bigint n) - : mVarName(name), mElementSize(size1), mDeclarationId(id) -{ - mNum.push_back(n); -} - -CheckBufferOverrun::ArrayInfo CheckBufferOverrun::ArrayInfo::limit(MathLib::bigint value) const -{ - const MathLib::bigint uvalue = std::max(MathLib::bigint(0), value); - MathLib::bigint n = 1; - for (std::size_t i = 0; i < mNum.size(); ++i) - n *= mNum[i]; - if (uvalue > n) - n = uvalue; - return ArrayInfo(mDeclarationId, mVarName, mElementSize, n - uvalue); -} - -MathLib::bigint CheckBufferOverrun::ArrayInfo::numberOfElements() const -{ - if (mNum.empty()) - return 0; - - // total number of elements of array.. - MathLib::bigint ret = 1; - for (std::size_t i = 0; i < mNum.size(); ++i) { - ret *= mNum[i]; - } - return ret; -} - -MathLib::bigint CheckBufferOverrun::ArrayInfo::totalIndex(const std::vector &indexes) const -{ - MathLib::bigint index = 0; - MathLib::bigint elements = 1; - for (std::size_t i = 0; i < mNum.size(); ++i) { - const std::size_t ri = mNum.size() - 1U - i; - index += indexes[ri].intvalue * elements; - elements *= mNum[ri]; - } - return index; -} - - -void CheckBufferOverrun::arrayIndexThenCheck() -{ - if (!mSettings->isEnabled(Settings::PORTABILITY)) - return; - - const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); - for (const Scope * const scope : symbolDatabase->functionScopes) { - for (const Token *tok = scope->bodyStart; tok && tok != scope->bodyEnd; tok = tok->next()) { - if (Token::simpleMatch(tok, "sizeof (")) { - tok = tok->linkAt(1); - continue; - } - - if (Token::Match(tok, "%name% [ %var% ]")) { - tok = tok->next(); - - const unsigned int indexID = tok->next()->varId(); - const std::string& indexName(tok->strAt(1)); - - // Iterate AST upwards - const Token* tok2 = tok; - const Token* tok3 = tok2; - while (tok2->astParent() && tok2->tokType() != Token::eLogicalOp) { - tok3 = tok2; - tok2 = tok2->astParent(); - } - - // Ensure that we ended at a logical operator and that we came from its left side - if (tok2->tokType() != Token::eLogicalOp || tok2->astOperand1() != tok3) - continue; - - // check if array index is ok - // statement can be closed in parentheses, so "(| " is using - if (Token::Match(tok2, "&& (| %varid% <|<=", indexID)) - arrayIndexThenCheckError(tok, indexName); - else if (Token::Match(tok2, "&& (| %any% >|>= %varid% !!+", indexID)) - arrayIndexThenCheckError(tok, indexName); + if (error) + bufferOverflowError(args[argnr]); } } } } -void CheckBufferOverrun::arrayIndexThenCheckError(const Token *tok, const std::string &indexName) +void CheckBufferOverrun::bufferOverflowError(const Token *tok) { - reportError(tok, Severity::style, "arrayIndexThenCheck", - "$symbol:" + indexName + "\n" - "Array index '$symbol' is used before limits check.\n" - "Defensive programming: The variable '$symbol' 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.", CWE398, false); -} - -std::string CheckBufferOverrun::MyFileInfo::toString() const -{ - std::ostringstream ret; - for (std::map::const_iterator it = arrayUsage.begin(); it != arrayUsage.end(); ++it) { - ret << " first) << '\"' - << " index=\"" << it->second.index << '\"' - << " fileName=\"" << ErrorLogger::toxml(it->second.fileName) << '\"' - << " linenr=\"" << it->second.linenr << "\"/>\n"; - } - for (std::map::const_iterator it = arraySize.begin(); it != arraySize.end(); ++it) { - ret << " first) << '\"' - << " size=\"" << it->second << "\"/>\n"; - } - return ret.str(); -} - -Check::FileInfo* CheckBufferOverrun::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const -{ - (void)settings; - - MyFileInfo *fileInfo = new MyFileInfo; - - // Array usage.. - const SymbolDatabase* const symbolDB = tokenizer->getSymbolDatabase(); - const std::size_t functions = symbolDB->functionScopes.size(); - for (std::size_t i = 0; i < functions; ++i) { - const Scope * const scope = symbolDB->functionScopes[i]; - for (const Token *tok = scope->bodyStart; tok && tok != scope->bodyEnd; tok = tok->next()) { - if (Token::Match(tok, "%var% [") && - Token::Match(tok->linkAt(1), "] !![") && - tok->variable() && - tok->variable()->isExtern() && - tok->variable()->isGlobal() && - tok->next()->astOperand2()) { - const ValueFlow::Value *value = tok->next()->astOperand2()->getMaxValue(false); - if (value && value->intvalue > 0) { - const MathLib::bigint arrayIndex = value->intvalue; - const std::map::iterator it = fileInfo->arrayUsage.find(tok->str()); - if (it != fileInfo->arrayUsage.end() && it->second.index >= arrayIndex) - continue; - struct MyFileInfo::ArrayUsage arrayUsage; - arrayUsage.index = arrayIndex; - arrayUsage.fileName = tokenizer->list.file(tok); - arrayUsage.linenr = tok->linenr(); - fileInfo->arrayUsage[tok->str()] = arrayUsage; - } - } - } - } - - // Arrays.. - const std::list &varlist = symbolDB->scopeList.front().varlist; - for (std::list::const_iterator it = varlist.begin(); it != varlist.end(); ++it) { - const Variable &var = *it; - if (!var.isStatic() && var.isArray() && var.dimensions().size() == 1U && var.dimension(0U) > 0U) - fileInfo->arraySize[var.name()] = var.dimension(0U); - } - - return fileInfo; -} - -Check::FileInfo * CheckBufferOverrun::loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const -{ - const std::string ArrayUsage("ArrayUsage"); - const std::string ArraySize("ArraySize"); - - MyFileInfo *fileInfo = new MyFileInfo; - for (const tinyxml2::XMLElement *e = xmlElement->FirstChildElement(); e; e = e->NextSiblingElement()) { - if (e->Name() == ArrayUsage) { - const char *array = e->Attribute("array"); - const char *arrayIndex = e->Attribute("index"); - const char *fileName = e->Attribute("fileName"); - const char *linenr = e->Attribute("linenr"); - if (!array || !arrayIndex || !MathLib::isInt(arrayIndex) || !fileName || !linenr || !MathLib::isInt(linenr)) - continue; - struct MyFileInfo::ArrayUsage arrayUsage; - arrayUsage.index = MathLib::toLongNumber(arrayIndex); - arrayUsage.fileName = fileName; - arrayUsage.linenr = MathLib::toLongNumber(linenr); - fileInfo->arrayUsage[array] = arrayUsage; - } else if (e->Name() == ArraySize) { - const char *array = e->Attribute("array"); - const char *size = e->Attribute("size"); - if (!array || !size || !MathLib::isInt(size)) - continue; - fileInfo->arraySize[array] = MathLib::toLongNumber(size); - } - } - - return fileInfo; -} - - -bool CheckBufferOverrun::analyseWholeProgram(const CTU::FileInfo *ctu, const std::list &fileInfo, const Settings&, ErrorLogger &errorLogger) -{ - (void)ctu; - bool errors = false; - // Merge all fileInfo - MyFileInfo all; - for (std::list::const_iterator it = fileInfo.begin(); it != fileInfo.end(); ++it) { - const MyFileInfo *fi = dynamic_cast(*it); - if (!fi) - continue; - - // merge array usage - for (std::map::const_iterator it2 = fi->arrayUsage.begin(); it2 != fi->arrayUsage.end(); ++it2) { - const std::map::const_iterator allit = all.arrayUsage.find(it2->first); - if (allit == all.arrayUsage.end() || it2->second.index > allit->second.index) - all.arrayUsage[it2->first] = it2->second; - } - - // merge array info - for (std::map::const_iterator it2 = fi->arraySize.begin(); it2 != fi->arraySize.end(); ++it2) { - const std::map::const_iterator allit = all.arraySize.find(it2->first); - if (allit == all.arraySize.end()) - all.arraySize[it2->first] = it2->second; - else - all.arraySize[it2->first] = -1; - } - } - - // Check buffer usage - for (std::map::const_iterator it = all.arrayUsage.begin(); it != all.arrayUsage.end(); ++it) { - const std::map::const_iterator sz = all.arraySize.find(it->first); - if (sz != all.arraySize.end() && sz->second > 0 && sz->second < it->second.index) { - ErrorLogger::ErrorMessage::FileLocation fileLoc; - fileLoc.setfile(it->second.fileName); - fileLoc.line = it->second.linenr; - - std::list locationList(1, fileLoc); - - std::ostringstream ostr; - ostr << "Array " << it->first << '[' << sz->second << "] accessed at index " << it->second.index << " which is out of bounds"; - - const ErrorLogger::ErrorMessage errmsg(locationList, - emptyString, - Severity::error, - ostr.str(), - "arrayIndexOutOfBounds", - CWE788, false); - errorLogger.reportErr(errmsg); - errors = true; - } - } - return errors; -} - -unsigned int CheckBufferOverrun::sizeOfType(const Token *type) const -{ - return mTokenizer->getSymbolDatabase()->sizeOfType(type); + reportError(tok, Severity::error, "bufferAccessOutOfBounds", "Buffer is accessed out of bounds: " + (tok ? tok->expressionString() : "buf"), CWE788, false); } diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index 14d4ac765..401237d3d 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -75,243 +75,42 @@ public: void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) OVERRIDE { CheckBufferOverrun checkBufferOverrun(tokenizer, settings, errorLogger); - checkBufferOverrun.checkGlobalAndLocalVariable(); - if (tokenizer && tokenizer->isMaxTime()) - return; - checkBufferOverrun.checkStructVariable(); - checkBufferOverrun.checkBufferAllocatedWithStrlen(); - checkBufferOverrun.checkInsecureCmdLineArgs(); - checkBufferOverrun.arrayIndexThenCheck(); - checkBufferOverrun.negativeArraySize(); + checkBufferOverrun.arrayIndex(); + checkBufferOverrun.bufferOverflow(); } void runChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) OVERRIDE { - CheckBufferOverrun checkBufferOverrun(tokenizer, settings, errorLogger); - checkBufferOverrun.bufferOverrun(); - checkBufferOverrun.checkStringArgument(); + (void)tokenizer; + (void)settings; + (void)errorLogger; } - /** @brief %Check for buffer overruns (single pass, use ast and valueflow) */ - void bufferOverrun(); - - /** @brief Using array index before bounds check */ - void arrayIndexThenCheck(); - - /** @brief negative size for array */ - void negativeArraySize(); - - /** - * @brief Get minimum length of format string result - * @param input_string format string - * @param parameters given parameters to sprintf - * @return minimum length of resulting string - */ - static MathLib::biguint countSprintfLength(const std::string &input_string, const std::list ¶meters); - - /** Check for buffer overruns - locate struct variables and check them with the .._CheckScope function */ - void checkStructVariable(); - - /** Check for buffer overruns - locate global variables and local function variables and check them with the checkScope function */ - void checkGlobalAndLocalVariable(); - - /** Check for buffer overruns due to allocating strlen(src) bytes instead of (strlen(src)+1) bytes before copying a string */ - void checkBufferAllocatedWithStrlen(); - - /** Check string argument buffer overruns */ - void checkStringArgument(); - - /** Check for buffer overruns due to copying command-line args to fixed-sized buffers without bounds checking */ - void checkInsecureCmdLineArgs(); - - /** Information about N-dimensional array */ - class CPPCHECKLIB ArrayInfo { - private: - /** number of elements of array */ - std::vector mNum; - - /** full name of variable as pattern */ - std::string mVarName; - - /** size of each element in array */ - MathLib::bigint mElementSize; - - /** declaration id */ - unsigned int mDeclarationId; - - public: - ArrayInfo(); - ArrayInfo(const Variable *var, const SymbolDatabase *symbolDatabase, const unsigned int forcedeclid = 0); - - /** - * Create array info with specified data - * The intention is that this is only a temporary solution.. all - * checking should be based on ArrayInfo from the start and then - * this will not be needed as the declare can be used instead. - */ - ArrayInfo(unsigned int id, const std::string &name, MathLib::bigint size1, MathLib::bigint n); - - /** Create a copy ArrayInfo where the number of elements have been limited by a value */ - ArrayInfo limit(MathLib::bigint value) const; - - /** array sizes */ - const std::vector &num() const { - return mNum; - } - - /** array size */ - MathLib::bigint num(std::size_t index) const { - return mNum[index]; - } - void num(std::size_t index, MathLib::bigint number) { - mNum[index] = number; - } - - /** size of each element */ - MathLib::bigint element_size() const { - return mElementSize; - } - - /** Variable name */ - unsigned int declarationId() const { - return mDeclarationId; - } - void declarationId(unsigned int id) { - mDeclarationId = id; - } - - /** Variable name */ - const std::string &varname() const { - return mVarName; - } - void varname(const std::string &name) { - mVarName = name; - } - - MathLib::bigint numberOfElements() const; - MathLib::bigint totalIndex(const std::vector &indexes) const; - }; - - /** Check for buffer overruns (based on ArrayInfo) */ - void checkScope(const Token *tok, const ArrayInfo &arrayInfo); - void checkScope(const Token *tok, std::map arrayInfos); - void checkScope_inner(const Token *tok, const ArrayInfo &arrayInfo); - - /** Check for buffer overruns */ - void checkScope(const Token *tok, const std::vector &varname, const ArrayInfo &arrayInfo); - - /** - * Helper function for checkFunctionCall - check a function parameter - * \param ftok token for the function name - * \param paramIndex on what parameter is the array used - * \param arrayInfo the array information - * \param callstack call stack. This is used to prevent recursion and to provide better error messages. Pass a empty list from checkScope etc. - */ - void checkFunctionParameter(const Token &ftok, const unsigned int paramIndex, const ArrayInfo &arrayInfo, const std::list& callstack); - - /** - * Helper function that checks if the array is used and if so calls the checkFunctionCall - * @param tok token that matches "%name% (" - * @param arrayInfo the array information - * \param callstack call stack. This is used to prevent recursion and to provide better error messages. Pass a empty list from checkScope etc. - */ - void checkFunctionCall(const Token *tok, const ArrayInfo &arrayInfo, std::list callstack); - - void arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector &index); - void arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector &index); - - /* data for multifile checking */ - class MyFileInfo : public Check::FileInfo { - public: - std::string toString() const OVERRIDE; - - struct ArrayUsage { - MathLib::bigint index; - std::string fileName; - unsigned int linenr; - }; - - /* key:arrayName */ - std::map arrayUsage; - - /* key:arrayName, data:arraySize */ - std::map arraySize; - }; - - /** @brief Parse current TU and extract file info */ - Check::FileInfo *getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const OVERRIDE; - - Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const OVERRIDE; - - /** @brief Analyse all file infos for all TU */ - bool analyseWholeProgram(const CTU::FileInfo *ctu, const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger) OVERRIDE; - - /** - * Calculates sizeof value for given type. - * @param type Token which will contain e.g. "int", "*", or string. - * @return sizeof for given type, or 0 if it can't be calculated. - */ - unsigned int sizeOfType(const Token *type) const; - -private: - static bool isArrayOfStruct(const Token* tok, int &position); - void arrayIndexOutOfBoundsError(const std::list &callstack, const ArrayInfo &arrayInfo, const std::vector &index); - void bufferOverrunError(const Token *tok, const std::string &varnames = emptyString); - void bufferOverrunError(const std::list &callstack, const std::string &varnames = emptyString); - void strncatUsageError(const Token *tok); - void negativeMemoryAllocationSizeError(const Token *tok); // provide a negative value to memory allocation function - void negativeArraySizeError(const Token *tok); - void outOfBoundsError(const Token *tok, const std::string &what, const bool show_size_info, const MathLib::bigint &supplied_size, const MathLib::bigint &actual_size); - void sizeArgumentAsCharError(const Token *tok); - void terminateStrncpyError(const Token *tok, const std::string &varname); - void bufferNotZeroTerminatedError(const Token *tok, const std::string &varname, const std::string &function); - void negativeIndexError(const Token *tok, MathLib::bigint index); - void negativeIndexError(const Token *tok, const ValueFlow::Value &index); - void cmdLineArgsError(const Token *tok); - void pointerOutOfBoundsError(const Token *tok, const Token *index=nullptr, const MathLib::bigint indexvalue=0); - void arrayIndexThenCheckError(const Token *tok, const std::string &indexName); - void possibleBufferOverrunError(const Token *tok, const std::string &src, const std::string &dst, bool cat); - void argumentSizeError(const Token *tok, const std::string &functionName, const std::string &varname); - - void valueFlowCheckArrayIndex(const Token * const tok, const ArrayInfo &arrayInfo); - -public: void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE { CheckBufferOverrun c(nullptr, settings, errorLogger); - const std::vector indexes(2, 1); - c.arrayIndexOutOfBoundsError(nullptr, ArrayInfo(0, "array", 1, 2), indexes); - c.bufferOverrunError(nullptr, std::string("buffer")); - c.strncatUsageError(nullptr); - c.outOfBoundsError(nullptr, "index", true, 2, 1); - c.sizeArgumentAsCharError(nullptr); - c.terminateStrncpyError(nullptr, "buffer"); - c.bufferNotZeroTerminatedError(nullptr, "buffer", "strncpy"); - c.negativeIndexError(nullptr, -1); - c.cmdLineArgsError(nullptr); - c.pointerOutOfBoundsError(nullptr, nullptr, 0); - c.arrayIndexThenCheckError(nullptr, "index"); - c.possibleBufferOverrunError(nullptr, "source", "destination", false); - c.argumentSizeError(nullptr, "function", "array"); - c.negativeMemoryAllocationSizeError(nullptr); - c.negativeArraySizeError(nullptr); - 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); + c.arrayIndexError(nullptr, nullptr, nullptr); + c.negativeIndexError(nullptr, nullptr, nullptr); } + private: + void arrayIndex(); + void arrayIndexError(const Token *tok, const Variable *var, const ValueFlow::Value *index); + void negativeIndexError(const Token *tok, const Variable *var, const ValueFlow::Value *negativeValue); + + void bufferOverflow(); + void bufferOverflowError(const Token *tok); + + size_t getBufferSize(const Token *bufTok) const; + static std::string myName() { return "Bounds checking"; } std::string classInfo() const OVERRIDE { return "Out of bounds checking:\n" - "- Array index out of bounds detection by value flow analysis\n" - "- Dangerous usage of strncat()\n" - "- char constant passed as size to function like memset()\n" - "- strncpy() leaving string unterminated\n" - "- Accessing array with negative index\n" - "- Unsafe usage of main(argv, argc) arguments\n" - "- Accessing array with index variable before checking its value\n" - "- Check for large enough arrays being passed to functions\n" - "- Allocating memory with a negative size\n"; + "- Array index out of bounds\n" + "- Buffer overflow\n" + "- Dangerous usage of strncat()\n"; } }; /// @} diff --git a/lib/library.cpp b/lib/library.cpp index f4ab94f65..4be26049e 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -648,13 +648,13 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co ArgumentChecks::MinSize::Type type; if (strcmp(typeattr,"strlen")==0) - type = ArgumentChecks::MinSize::STRLEN; + type = ArgumentChecks::MinSize::Type::STRLEN; else if (strcmp(typeattr,"argvalue")==0) - type = ArgumentChecks::MinSize::ARGVALUE; + type = ArgumentChecks::MinSize::Type::ARGVALUE; else if (strcmp(typeattr,"sizeof")==0) - type = ArgumentChecks::MinSize::SIZEOF; + type = ArgumentChecks::MinSize::Type::SIZEOF; else if (strcmp(typeattr,"mul")==0) - type = ArgumentChecks::MinSize::MUL; + type = ArgumentChecks::MinSize::Type::MUL; else return Error(BAD_ATTRIBUTE_VALUE, typeattr); @@ -664,9 +664,9 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co if (strlen(argattr) != 1 || argattr[0]<'0' || argattr[0]>'9') return Error(BAD_ATTRIBUTE_VALUE, argattr); - ac.minsizes.reserve(type == ArgumentChecks::MinSize::MUL ? 2 : 1); + ac.minsizes.reserve(type == ArgumentChecks::MinSize::Type::MUL ? 2 : 1); ac.minsizes.emplace_back(type,argattr[0]-'0'); - if (type == ArgumentChecks::MinSize::MUL) { + if (type == ArgumentChecks::MinSize::Type::MUL) { const char *arg2attr = argnode->Attribute("arg2"); if (!arg2attr) return Error(MISSING_ATTRIBUTE, "arg2"); @@ -1118,9 +1118,11 @@ int Library::returnValueContainer(const Token *ftok) const return it != mReturnValueContainer.end() ? it->second : -1; } -bool Library::hasminsize(const std::string &functionName) const +bool Library::hasminsize(const Token *ftok) const { - const std::map::const_iterator it1 = functions.find(functionName); + if (isNotLibraryFunction(ftok)) + return false; + const std::map::const_iterator it1 = functions.find(getFunctionName(ftok)); if (it1 == functions.cend()) return false; for (std::map::const_iterator it2 = it1->second.argumentChecks.cbegin(); it2 != it1->second.argumentChecks.cend(); ++it2) { diff --git a/lib/library.h b/lib/library.h index 1fa4139fa..cd25eb254 100644 --- a/lib/library.h +++ b/lib/library.h @@ -322,7 +322,7 @@ public: return arg && arg->iteratorInfo.it ? &arg->iteratorInfo : nullptr; } - bool hasminsize(const std::string &functionName) const; + bool hasminsize(const Token *ftok) const; const std::vector *argminsizes(const Token *ftok, int argnr) const { const ArgumentChecks *arg = getarg(ftok, argnr); diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 202fb09d2..56d2e9a4e 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -90,9 +90,10 @@ private: TEST_CASE(array_index_1); TEST_CASE(array_index_2); TEST_CASE(array_index_3); + // TODO string TEST_CASE(array_index_4); TEST_CASE(array_index_6); TEST_CASE(array_index_7); - TEST_CASE(array_index_9); + // TODO CTU TEST_CASE(array_index_9); TEST_CASE(array_index_11); TEST_CASE(array_index_12); TEST_CASE(array_index_13); @@ -116,7 +117,7 @@ private: TEST_CASE(array_index_31); // ticket #2120 - out of bounds in subfunction when type is unknown TEST_CASE(array_index_32); TEST_CASE(array_index_33); // ticket #3044 - TEST_CASE(array_index_34); // ticket #3063 + // TODO multidim TEST_CASE(array_index_34); // ticket #3063 TEST_CASE(array_index_35); // ticket #2889 TEST_CASE(array_index_36); // ticket #2960 TEST_CASE(array_index_37); @@ -124,13 +125,13 @@ private: TEST_CASE(array_index_39); TEST_CASE(array_index_40); // loop variable calculation, taking address TEST_CASE(array_index_41); // structs with the same name - TEST_CASE(array_index_42); + // TODO malloc TEST_CASE(array_index_42); TEST_CASE(array_index_43); // struct with array TEST_CASE(array_index_44); // #3979 TEST_CASE(array_index_45); // #4207 - calling function with variable number of parameters (...) TEST_CASE(array_index_46); // #4840 - two-statement for loop TEST_CASE(array_index_47); // #5849 - TEST_CASE(array_index_multidim); + // TODO multidim TEST_CASE(array_index_multidim); TEST_CASE(array_index_switch_in_for); TEST_CASE(array_index_for_in_for); // FP: #2634 TEST_CASE(array_index_calculation); @@ -148,7 +149,7 @@ private: TEST_CASE(array_index_vla_for); // #3221: access VLA inside for TEST_CASE(array_index_extern); // FP when using 'extern'. #1684 TEST_CASE(array_index_cast); // FP after cast. #2841 - TEST_CASE(array_index_string_literal); + // TODO string pointer TEST_CASE(array_index_string_literal); TEST_CASE(array_index_same_struct_and_var_name); // #4751 - not handled well when struct name and var name is same TEST_CASE(array_index_valueflow); TEST_CASE(array_index_valueflow_pointer); @@ -159,7 +160,7 @@ private: TEST_CASE(buffer_overrun_3); TEST_CASE(buffer_overrun_4); TEST_CASE(buffer_overrun_5); - TEST_CASE(buffer_overrun_6); + // TODO strcat TEST_CASE(buffer_overrun_6); TEST_CASE(buffer_overrun_7); TEST_CASE(buffer_overrun_8); TEST_CASE(buffer_overrun_9); @@ -173,12 +174,11 @@ private: TEST_CASE(buffer_overrun_24); // index variable is changed in for-loop TEST_CASE(buffer_overrun_26); // #4432 (segmentation fault) TEST_CASE(buffer_overrun_27); // #4444 (segmentation fault) - TEST_CASE(buffer_overrun_28); // Out of bound char array access TEST_CASE(buffer_overrun_29); // #7083: false positive: typedef and initialization with strings TEST_CASE(buffer_overrun_30); // #6367 - TEST_CASE(buffer_overrun_bailoutIfSwitch); // ticket #2378 : bailoutIfSwitch - TEST_CASE(buffer_overrun_function_array_argument); - TEST_CASE(possible_buffer_overrun_1); // #3035 + // TODO CTU TEST_CASE(buffer_overrun_bailoutIfSwitch); // ticket #2378 : bailoutIfSwitch + // TODO TEST_CASE(buffer_overrun_function_array_argument); + // TODO alloca TEST_CASE(possible_buffer_overrun_1); // #3035 TEST_CASE(buffer_overrun_readSizeFromCfg); TEST_CASE(valueflow_string); // using ValueFlow string values in checking @@ -188,41 +188,41 @@ private: // char a[10]; // char *p1 = a + 10; // OK // char *p2 = a + 11 // UB - TEST_CASE(pointer_out_of_bounds_1); - TEST_CASE(pointer_out_of_bounds_2); - TEST_CASE(pointer_out_of_bounds_3); - TEST_CASE(pointer_out_of_bounds_sub); + // TODO TEST_CASE(pointer_out_of_bounds_1); + // TODO TEST_CASE(pointer_out_of_bounds_2); + // TODO TEST_CASE(pointer_out_of_bounds_3); + // TODO TEST_CASE(pointer_out_of_bounds_sub); - TEST_CASE(strncat1); - TEST_CASE(strncat2); - TEST_CASE(strncat3); + // TODO TEST_CASE(strncat1); + // TODO TEST_CASE(strncat2); + // TODO TEST_CASE(strncat3); - TEST_CASE(strcat1); - TEST_CASE(strcat2); - TEST_CASE(strcat3); + // TODO TEST_CASE(strcat1); + // TODO TEST_CASE(strcat2); + // TODO TEST_CASE(strcat3); TEST_CASE(varid1); TEST_CASE(varid2); // ticket #4764 TEST_CASE(assign1); - TEST_CASE(alloc_new); // Buffer allocated with new - TEST_CASE(alloc_malloc); // Buffer allocated with malloc - TEST_CASE(alloc_string); // statically allocated buffer - TEST_CASE(alloc_alloca); // Buffer allocated with alloca + // TODO TEST_CASE(alloc_new); // Buffer allocated with new + // TODO TEST_CASE(alloc_malloc); // Buffer allocated with malloc + // TODO TEST_CASE(alloc_string); // statically allocated buffer + // TODO TEST_CASE(alloc_alloca); // Buffer allocated with alloca - TEST_CASE(countSprintfLength); + // TODO TEST_CASE(countSprintfLength); TEST_CASE(minsize_argvalue); TEST_CASE(minsize_sizeof); TEST_CASE(minsize_strlen); TEST_CASE(minsize_mul); TEST_CASE(unknownType); - TEST_CASE(terminateStrncpy1); - TEST_CASE(terminateStrncpy2); - TEST_CASE(terminateStrncpy3); - TEST_CASE(terminateStrncpy4); - TEST_CASE(recursive_long_time); + // TODO strncpy TEST_CASE(terminateStrncpy1); + // TODO strncpy TEST_CASE(terminateStrncpy2); + // TODO strncpy TEST_CASE(terminateStrncpy3); + // TODO strncpy TEST_CASE(terminateStrncpy4); + // TODO strncpy TEST_CASE(recursive_long_time); TEST_CASE(crash1); // Ticket #1587 - crash TEST_CASE(crash2); // Ticket #3034 - crash @@ -231,25 +231,19 @@ private: TEST_CASE(crash5); // Ticket #8644 - crash TEST_CASE(crash6); // Ticket #9024 - crash - TEST_CASE(executionPaths1); - TEST_CASE(executionPaths2); - TEST_CASE(executionPaths3); // no FP for function parameter - TEST_CASE(executionPaths5); // Ticket #2920 - False positive when size is unknown - TEST_CASE(executionPaths6); // unknown types - - TEST_CASE(insecureCmdLineArgs); - TEST_CASE(checkBufferAllocatedWithStrlen); + // TODO TEST_CASE(insecureCmdLineArgs); + // TODO TEST_CASE(checkBufferAllocatedWithStrlen); TEST_CASE(scope); // handling different scopes TEST_CASE(getErrorMessages); // Access array and then check if the used index is within bounds - TEST_CASE(arrayIndexThenCheck); + // TODO TEST_CASE(arrayIndexThenCheck); - TEST_CASE(bufferNotZeroTerminated); + // TODO TEST_CASE(bufferNotZeroTerminated); - TEST_CASE(negativeMemoryAllocationSizeError) // #389 + // TODO TEST_CASE(negativeMemoryAllocationSizeError) // #389 TEST_CASE(negativeArraySize); } @@ -360,6 +354,10 @@ private: " return y;\n" "}"); ASSERT_EQUALS("", errout.str()); + + check("int x[5];\n" + "int a = x[10];\n"); + ASSERT_EQUALS("[test.cpp:2]: (error) Array 'x[5]' accessed at index 10, which is out of bounds.\n", errout.str()); } @@ -373,6 +371,16 @@ private: TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Array 'str[16]' accessed at index 16, which is out of bounds.\n", "", errout.str()); } + void array_index_4() { + check("char c = \"abc\"[4];"); + ASSERT_EQUALS("[test.cpp:1]: (error) Array index out of bounds: \"abc\"\n", errout.str()); + + check("p = &\"abc\"[4];"); + ASSERT_EQUALS("", errout.str()); + + check("char c = \"\\0abc\"[2];"); + ASSERT_EQUALS("", errout.str()); + } void array_index_3() { check("void f()\n" @@ -477,8 +485,7 @@ private: " struct ABC* x = malloc(sizeof(struct ABC) + 10);\n" " x->str[1] = 0;" "}"); - ASSERT_EQUALS("[test.cpp:10]: (error) Array 'x->str[1]' accessed at index 1, which is out of bounds.\n" - "[test.cpp:10]: (error) Array 'x.str[1]' accessed at index 1, which is out of bounds.\n", errout.str()); + TODO_ASSERT_EQUALS("error", "", errout.str()); // This is not out of bounds because it is a variable length array // and the index is within the memory allocated. @@ -561,7 +568,7 @@ private: " struct ABC x;\n" " x.str[1] = 0;" "}"); - ASSERT_EQUALS("[test.cpp:9]: (error) Array 'x.str[1]' accessed at index 1, which is out of bounds.\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:9]: (error) Array 'x.str[1]' accessed at index 1, which is out of bounds.\n", "", errout.str()); check("struct foo\n" "{\n" @@ -600,11 +607,11 @@ private: "{\n" " abc->str[10] = 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:8]: (error) Array 'abc->str[10]' accessed at index 10, which is out of bounds.\n" - "[test.cpp:8]: (error) Array 'abc.str[10]' accessed at index 10, which is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:8]: (error) Array 'abc->str[10]' accessed at index 10, which is out of bounds.\n", errout.str()); } void array_index_9() { + // Cross translation unit analysis check("static void memclr( char *data )\n" "{\n" " data[10] = 0;\n" @@ -688,8 +695,7 @@ private: " abc->str[10] = 0;\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:13]: (error) Array 'abc->str[10]' accessed at index 10, which is out of bounds.\n" - "[test.cpp:13]: (error) Array 'abc.str[10]' accessed at index 10, which is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:13]: (error) Array 'abc->str[10]' accessed at index 10, which is out of bounds.\n", errout.str()); } void array_index_12() { @@ -931,7 +937,7 @@ private: " a[-1] = 0;\n" // negative index " a[" + charMaxPlusOne.str() + "] = 0;\n" // 128/256 > CHAR_MAX "}\n").c_str()); - ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n" + ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a["+charMaxPlusOne.str()+"]' accessed at index -1, which is out of bounds.\n" "[test.cpp:4]: (error) Array 'a["+charMaxPlusOne.str()+"]' accessed at index "+charMaxPlusOne.str()+", which is out of bounds.\n", errout.str()); check("void f(signed char n) {\n" @@ -939,7 +945,7 @@ private: " a[-1] = 0;\n" // negative index " a[128] = 0;\n" // 128 > SCHAR_MAX "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n" + ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[128]' accessed at index -1, which is out of bounds.\n" "[test.cpp:4]: (error) Array 'a[128]' accessed at index 128, which is out of bounds.\n", errout.str()); check("void f(unsigned char n) {\n" @@ -947,7 +953,7 @@ private: " a[-1] = 0;\n" // negative index " a[256] = 0;\n" // 256 > UCHAR_MAX "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n" + ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[256]' accessed at index -1, which is out of bounds.\n" "[test.cpp:4]: (error) Array 'a[256]' accessed at index 256, which is out of bounds.\n", errout.str()); check("void f(short n) {\n" @@ -955,7 +961,7 @@ private: " a[-1] = 0;\n" // negative index " a[32768] = 0;\n" // 32768 > SHRT_MAX "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n" + ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[32768]' accessed at index -1, which is out of bounds.\n" "[test.cpp:4]: (error) Array 'a[32768]' accessed at index 32768, which is out of bounds.\n", errout.str()); check("void f(unsigned short n) {\n" @@ -963,7 +969,7 @@ private: " a[-1] = 0;\n" // negative index " a[65536] = 0;\n" // 65536 > USHRT_MAX "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n" + ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[65536]' accessed at index -1, which is out of bounds.\n" "[test.cpp:4]: (error) Array 'a[65536]' accessed at index 65536, which is out of bounds.\n", errout.str()); check("void f(signed short n) {\n" @@ -971,26 +977,26 @@ private: " a[-1] = 0;\n" // negative index " a[32768] = 0;\n" // 32768 > SHRT_MAX "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n" + ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[32768]' accessed at index -1, which is out of bounds.\n" "[test.cpp:4]: (error) Array 'a[32768]' accessed at index 32768, which is out of bounds.\n", errout.str()); check("void f(int n) {\n" " int a[n];\n" // n <= INT_MAX " a[-1] = 0;\n" // negative index "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[2147483648]' accessed at index -1, which is out of bounds.\n", errout.str()); check("void f(unsigned int n) {\n" " int a[n];\n" // n <= UINT_MAX " a[-1] = 0;\n" // negative index "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[4294967296]' accessed at index -1, which is out of bounds.\n", errout.str()); check("void f(signed int n) {\n" " int a[n];\n" // n <= INT_MAX " a[-1] = 0;\n" // negative index "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[2147483648]' accessed at index -1, which is out of bounds.\n", errout.str()); } void array_index_25() { // #1536 @@ -1026,7 +1032,7 @@ private: " for (int i = 0; i < 10; i++)\n" " a[i-1] = a[i];\n" "}"); - ASSERT_EQUALS("[test.cpp:5]: (error) Array index -1 is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n", errout.str()); } void array_index_28() { @@ -1089,7 +1095,7 @@ private: " struct s1 obj;\n" " x(obj.delay, 123);\n" "}"); - ASSERT_EQUALS("[test.cpp:11] -> [test.cpp:6]: (error) Array 'obj.delay[3]' accessed at index 4, which is out of bounds.\n", errout.str()); + // TODO CTU ASSERT_EQUALS("[test.cpp:11] -> [test.cpp:6]: (error) Array 'obj.delay[3]' accessed at index 4, which is out of bounds.\n", errout.str()); check("struct s1 {\n" " float a[0];\n" @@ -1112,7 +1118,7 @@ private: " }\n" " int m_x[1];\n" "};"); - ASSERT_EQUALS("[test.cpp:7]: (error) Array 'm_x[1]' accessed at index 1, which is out of bounds.\n", errout.str()); + // TODO [1] ASSERT_EQUALS("[test.cpp:7]: (error) Array 'm_x[1]' accessed at index 1, which is out of bounds.\n", errout.str()); } void array_index_33() { @@ -1185,14 +1191,14 @@ private: " struct Struct { unsigned m_Var[1]; } s;\n" " s.m_Var[1] = 1;\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array 's.m_Var[1]' accessed at index 1, which is out of bounds.\n", errout.str()); + // TODO ASSERT_EQUALS("[test.cpp:3]: (error) Array 's.m_Var[1]' accessed at index 1, which is out of bounds.\n", errout.str()); check("struct Struct { unsigned m_Var[1]; };\n" "void f() {\n" " struct Struct s;\n" " s.m_Var[1] = 1;\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Array 's.m_Var[1]' accessed at index 1, which is out of bounds.\n", errout.str()); + // TODO ASSERT_EQUALS("[test.cpp:4]: (error) Array 's.m_Var[1]' accessed at index 1, which is out of bounds.\n", errout.str()); check("struct Struct { unsigned m_Var[1]; };\n" "void f() {\n" @@ -1354,8 +1360,7 @@ private: " y = var[ 0 ].arr[ 3 ];\n" // <-- array access out of bounds " return y;\n" "}"); - ASSERT_EQUALS("[test.cpp:10]: (error) Array 'var[0].arr[3]' accessed at index 3, which is out of bounds.\n" - "[test.cpp:10]: (error) Array 'var.arr[3]' accessed at index 3, which is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:10]: (error) Array 'var[0].arr[3]' accessed at index 3, which is out of bounds.\n", errout.str()); check("int f( )\n" "{\n" @@ -1399,8 +1404,7 @@ private: "var[0].var[ 2 ] = 2;\n" "var[0].var[ 4 ] = 4;\n" // <-- array access out of bounds "}"); - ASSERT_EQUALS("[test.cpp:9]: (error) Array 'var[0].var[3]' accessed at index 4, which is out of bounds.\n" - "[test.cpp:9]: (error) Array 'var.var[3]' accessed at index 4, which is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:9]: (error) Array 'var[0].var[3]' accessed at index 4, which is out of bounds.\n", errout.str()); check("void f( ) {\n" "struct S{\n" @@ -1436,7 +1440,7 @@ private: " int * p = &ab[10].a[0]; \n" " return 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Array 'ab[1]' accessed at index 10, which is out of bounds.\n", errout.str()); + // TODO ASSERT_EQUALS("[test.cpp:4]: (error) Array 'ab[1]' accessed at index 10, which is out of bounds.\n", errout.str()); } void array_index_44() { // #3979 (false positive) @@ -1744,14 +1748,14 @@ private: " char data[8];\n" " data[-1] = 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Array index -1 is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Array 'data[8]' accessed at index -1, which is out of bounds.\n", errout.str()); check("void f()\n" "{\n" " char data[8][4];\n" " data[5][-1] = 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Array index -1 is out of bounds.\n", errout.str()); + // TODO multidim ASSERT_EQUALS("[test.cpp:4]: (error) Array index -1 is out of bounds.\n", errout.str()); // #1614 - negative index is ok for pointers check("void foo(char *p)\n" @@ -1827,8 +1831,8 @@ private: " val[i+1] = val[i];\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:5]: (error) Array index -9994 is out of bounds.\n" - "[test.cpp:5]: (error) Array index -9995 is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5]: (error) Array 'val[5]' accessed at index -9994, which is out of bounds.\n" + "[test.cpp:5]: (error) Array 'val[5]' accessed at index -9995, which is out of bounds.\n", errout.str()); } @@ -1907,7 +1911,7 @@ private: " a[i - 1] = 0;\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:7]: (error) Array index -1 is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:7]: (error) Array 'a[2]' accessed at index -1, which is out of bounds.\n", errout.str()); } void array_index_for() { @@ -1981,7 +1985,7 @@ private: " a[i-1] = 0;\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:5]: (error) Array index -1 is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n", errout.str()); } void array_index_for_varid0() { // #4228: No varid for counter variable @@ -2031,7 +2035,7 @@ private: " char x[2];\n" " f1(x);\n" "}"); - ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:2]: (error) Array 'x[2]' accessed at index 4, which is out of bounds.\n", errout.str()); + // TODO CTU ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:2]: (error) Array 'x[2]' accessed at index 4, which is out of bounds.\n", errout.str()); } void array_index_string_literal() { @@ -2097,8 +2101,7 @@ private: " struct tt *tt=x;\n" " tt->name[22] = 123;\n" "}"); - ASSERT_EQUALS("[test.cpp:7]: (error) Array 'tt->name[21]' accessed at index 22, which is out of bounds.\n" - "[test.cpp:7]: (error) Array 'tt.name[21]' accessed at index 22, which is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:7]: (error) Array 'tt->name[21]' accessed at index 22, which is out of bounds.\n", errout.str()); } void array_index_valueflow() { @@ -2107,7 +2110,7 @@ private: " str[i] = 0;\n" " if (i==10) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Either the condition 'i==10' is redundant or the array 'str[3]' is accessed at index 10, which is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (warning) Either the condition 'i==10' is redundant or the array 'str[3]' is accessed at index 10, which is out of bounds.\n", errout.str()); check("void f(int i) {\n" " char str[3];\n" @@ -2116,7 +2119,7 @@ private: " case 10: break;\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (warning) Either the switch case 'case 10' is redundant or the array 'str[3]' is accessed at index 10, which is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:3]: (warning) Either the switch case 'case 10' is redundant or the array 'str[3]' is accessed at index 10, which is out of bounds.\n", errout.str()); check("void f() {\n" " char str[3];\n" @@ -2146,7 +2149,7 @@ private: " int *p = a;\n" " p[20] = 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Array 'a[10]' accessed at index 20, which is out of bounds.\n", errout.str()); + // TODO pointer ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Array 'a[10]' accessed at index 20, which is out of bounds.\n", errout.str()); { // address of @@ -2155,7 +2158,7 @@ private: " int *p = a;\n" " p[10] = 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Array 'a[10]' accessed at index 10, which is out of bounds.\n", errout.str()); + // TODO pointer ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Array 'a[10]' accessed at index 10, which is out of bounds.\n", errout.str()); check("void f() {\n" " int a[10];\n" @@ -2190,7 +2193,7 @@ private: " a += 4;\n" " a[-1] = 0;\n" "}"); - ASSERT_EQUALS("", errout.str()); + // TODO ASSERT_EQUALS("", errout.str()); } void array_index_enum_array() { // #8439 @@ -2212,7 +2215,7 @@ private: "{\n" " strcpy( abc->str, \"abcdef\" );\n" "}"); - ASSERT_EQUALS("[test.cpp:8]: (error) Buffer is accessed out of bounds: abc.str\n", errout.str()); + ASSERT_EQUALS("[test.cpp:8]: (error) Buffer is accessed out of bounds: abc->str\n", errout.str()); check("struct ABC\n" "{\n" @@ -2257,7 +2260,7 @@ private: " strcpy( abc->str, \"abcdef\" );\n" " free(abc);\n" "}"); - ASSERT_EQUALS("[test.cpp:8]: (error) Buffer is accessed out of bounds: abc.str\n", errout.str()); + ASSERT_EQUALS("[test.cpp:8]: (error) Buffer is accessed out of bounds: abc->str\n", errout.str()); } @@ -2543,18 +2546,6 @@ private: ASSERT_EQUALS("", errout.str()); } - void buffer_overrun_28() { - check("char c = \"abc\"[4];"); - ASSERT_EQUALS("[test.cpp:1]: (error) Buffer is accessed out of bounds: \"abc\"\n", errout.str()); - - check("p = &\"abc\"[4];"); - ASSERT_EQUALS("", errout.str()); - - check("char c = \"\\0abc\"[2];"); - ASSERT_EQUALS("", errout.str()); - } - - // #7083: false positive: typedef and initialization with strings void buffer_overrun_29() { check("typedef char testChar[10]; \n" @@ -2574,9 +2565,7 @@ private: " return s->m[sizeof(s->m)];\n" "}\n" ); - ASSERT_EQUALS("[test.cpp:3]: (error) Array 's->m[9]' accessed at index 36, which is out of bounds.\n" - "[test.cpp:3]: (error) Array 's.m[9]' accessed at index 36, which is out of bounds.\n", - errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Array 's->m[9]' accessed at index 36, which is out of bounds.\n", errout.str()); } @@ -2778,7 +2767,7 @@ private: " if (cond) x = \"abcde\";\n" " return x[20];\n" // <- array index out of bounds when x is "abcde" "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Array 'x[6]' accessed at index 20, which is out of bounds.\n", errout.str()); + // TODO ASSERT_EQUALS("[test.cpp:4]: (error) Array 'x[6]' accessed at index 20, which is out of bounds.\n", errout.str()); } void pointer_out_of_bounds_1() { @@ -2987,8 +2976,7 @@ private: " Fred *f; f = new Fred;\n" " return f->c[10];\n" "}"); - ASSERT_EQUALS("[test.cpp:5]: (error) Array 'f->c[10]' accessed at index 10, which is out of bounds.\n" - "[test.cpp:5]: (error) Array 'f.c[10]' accessed at index 10, which is out of bounds.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5]: (error) Array 'f->c[10]' accessed at index 10, which is out of bounds.\n", errout.str()); check("static const size_t MAX_SIZE = UNAVAILABLE_TO_CPPCHECK;\n" "struct Thing { char data[MAX_SIZE]; };\n" @@ -3154,78 +3142,78 @@ private: "}"); ASSERT_EQUALS("[test.cpp:4]: (error) Array 's[10]' accessed at index 10, which is out of bounds.\n", errout.str()); } + /* + void countSprintfLength() const { + std::list unknownParameter(1, nullptr); - void countSprintfLength() const { - std::list unknownParameter(1, nullptr); + ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("Hello", unknownParameter)); + ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("s", unknownParameter)); + ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("i", unknownParameter)); + ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("%d", unknownParameter)); + ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("%1d", unknownParameter)); + ASSERT_EQUALS(3, CheckBufferOverrun::countSprintfLength("%2.2d", unknownParameter)); + ASSERT_EQUALS(1, CheckBufferOverrun::countSprintfLength("%s", unknownParameter)); + ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("f%s", unknownParameter)); + ASSERT_EQUALS(1, CheckBufferOverrun::countSprintfLength("%-s", unknownParameter)); + ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%-5s", unknownParameter)); + ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("\\\"", unknownParameter)); + ASSERT_EQUALS(7, CheckBufferOverrun::countSprintfLength("Hello \\0Text", unknownParameter)); + ASSERT_EQUALS(1, CheckBufferOverrun::countSprintfLength("\\0", unknownParameter)); + ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("%%", unknownParameter)); + ASSERT_EQUALS(3, CheckBufferOverrun::countSprintfLength("%d%d", unknownParameter)); + ASSERT_EQUALS(3, CheckBufferOverrun::countSprintfLength("\\\\a%s\\0a", unknownParameter)); + ASSERT_EQUALS(10, CheckBufferOverrun::countSprintfLength("\\\\\\\\Hello%d \\0Text\\\\\\\\", unknownParameter)); + ASSERT_EQUALS(4, CheckBufferOverrun::countSprintfLength("%%%%%d", unknownParameter)); - ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("Hello", unknownParameter)); - ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("s", unknownParameter)); - ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("i", unknownParameter)); - ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("%d", unknownParameter)); - ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("%1d", unknownParameter)); - ASSERT_EQUALS(3, CheckBufferOverrun::countSprintfLength("%2.2d", unknownParameter)); - ASSERT_EQUALS(1, CheckBufferOverrun::countSprintfLength("%s", unknownParameter)); - ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("f%s", unknownParameter)); - ASSERT_EQUALS(1, CheckBufferOverrun::countSprintfLength("%-s", unknownParameter)); - ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%-5s", unknownParameter)); - ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("\\\"", unknownParameter)); - ASSERT_EQUALS(7, CheckBufferOverrun::countSprintfLength("Hello \\0Text", unknownParameter)); - ASSERT_EQUALS(1, CheckBufferOverrun::countSprintfLength("\\0", unknownParameter)); - ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("%%", unknownParameter)); - ASSERT_EQUALS(3, CheckBufferOverrun::countSprintfLength("%d%d", unknownParameter)); - ASSERT_EQUALS(3, CheckBufferOverrun::countSprintfLength("\\\\a%s\\0a", unknownParameter)); - ASSERT_EQUALS(10, CheckBufferOverrun::countSprintfLength("\\\\\\\\Hello%d \\0Text\\\\\\\\", unknownParameter)); - ASSERT_EQUALS(4, CheckBufferOverrun::countSprintfLength("%%%%%d", unknownParameter)); + Token strTok; + std::list stringAsParameter(1, &strTok); + strTok.str("\"\""); + ASSERT_EQUALS(4, CheckBufferOverrun::countSprintfLength("str%s", stringAsParameter)); + strTok.str("\"12345\""); + ASSERT_EQUALS(9, CheckBufferOverrun::countSprintfLength("str%s", stringAsParameter)); + ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%-4s", stringAsParameter)); + ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%-5s", stringAsParameter)); + ASSERT_EQUALS(7, CheckBufferOverrun::countSprintfLength("%-6s", stringAsParameter)); + ASSERT_EQUALS(5, CheckBufferOverrun::countSprintfLength("%.4s", stringAsParameter)); + ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%.5s", stringAsParameter)); + ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%.6s", stringAsParameter)); + ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%5.6s", stringAsParameter)); + ASSERT_EQUALS(7, CheckBufferOverrun::countSprintfLength("%6.6s", stringAsParameter)); - Token strTok; - std::list stringAsParameter(1, &strTok); - strTok.str("\"\""); - ASSERT_EQUALS(4, CheckBufferOverrun::countSprintfLength("str%s", stringAsParameter)); - strTok.str("\"12345\""); - ASSERT_EQUALS(9, CheckBufferOverrun::countSprintfLength("str%s", stringAsParameter)); - ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%-4s", stringAsParameter)); - ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%-5s", stringAsParameter)); - ASSERT_EQUALS(7, CheckBufferOverrun::countSprintfLength("%-6s", stringAsParameter)); - ASSERT_EQUALS(5, CheckBufferOverrun::countSprintfLength("%.4s", stringAsParameter)); - ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%.5s", stringAsParameter)); - ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%.6s", stringAsParameter)); - ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%5.6s", stringAsParameter)); - ASSERT_EQUALS(7, CheckBufferOverrun::countSprintfLength("%6.6s", stringAsParameter)); + Token numTok; + numTok.str("12345"); + std::list intAsParameter(1, &numTok); + ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%02ld", intAsParameter)); + ASSERT_EQUALS(9, CheckBufferOverrun::countSprintfLength("%08ld", intAsParameter)); + ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%.2d", intAsParameter)); + ASSERT_EQUALS(9, CheckBufferOverrun::countSprintfLength("%08.2d", intAsParameter)); + TODO_ASSERT_EQUALS(5, 2, CheckBufferOverrun::countSprintfLength("%x", intAsParameter)); + ASSERT_EQUALS(5, CheckBufferOverrun::countSprintfLength("%4x", intAsParameter)); + ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%5x", intAsParameter)); + ASSERT_EQUALS(5, CheckBufferOverrun::countSprintfLength("%.4x", intAsParameter)); + ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%.5x", intAsParameter)); + ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%1.5x", intAsParameter)); + ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%5.1x", intAsParameter)); - Token numTok; - numTok.str("12345"); - std::list intAsParameter(1, &numTok); - ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%02ld", intAsParameter)); - ASSERT_EQUALS(9, CheckBufferOverrun::countSprintfLength("%08ld", intAsParameter)); - ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%.2d", intAsParameter)); - ASSERT_EQUALS(9, CheckBufferOverrun::countSprintfLength("%08.2d", intAsParameter)); - TODO_ASSERT_EQUALS(5, 2, CheckBufferOverrun::countSprintfLength("%x", intAsParameter)); - ASSERT_EQUALS(5, CheckBufferOverrun::countSprintfLength("%4x", intAsParameter)); - ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%5x", intAsParameter)); - ASSERT_EQUALS(5, CheckBufferOverrun::countSprintfLength("%.4x", intAsParameter)); - ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%.5x", intAsParameter)); - ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%1.5x", intAsParameter)); - ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%5.1x", intAsParameter)); + Token floatTok; + floatTok.str("1.12345f"); + std::list floatAsParameter(1, &floatTok); + TODO_ASSERT_EQUALS(5, 3, CheckBufferOverrun::countSprintfLength("%.2f", floatAsParameter)); + ASSERT_EQUALS(9, CheckBufferOverrun::countSprintfLength("%8.2f", floatAsParameter)); + TODO_ASSERT_EQUALS(5, 3, CheckBufferOverrun::countSprintfLength("%2.2f", floatAsParameter)); - Token floatTok; - floatTok.str("1.12345f"); - std::list floatAsParameter(1, &floatTok); - TODO_ASSERT_EQUALS(5, 3, CheckBufferOverrun::countSprintfLength("%.2f", floatAsParameter)); - ASSERT_EQUALS(9, CheckBufferOverrun::countSprintfLength("%8.2f", floatAsParameter)); - TODO_ASSERT_EQUALS(5, 3, CheckBufferOverrun::countSprintfLength("%2.2f", floatAsParameter)); - - Token floatTok2; - floatTok2.str("100.12345f"); - std::list floatAsParameter2(1, &floatTok2); - TODO_ASSERT_EQUALS(7, 3, CheckBufferOverrun::countSprintfLength("%2.2f", floatAsParameter2)); - TODO_ASSERT_EQUALS(7, 3, CheckBufferOverrun::countSprintfLength("%.2f", floatAsParameter)); - TODO_ASSERT_EQUALS(7, 5, CheckBufferOverrun::countSprintfLength("%4.2f", floatAsParameter)); - - std::list multipleParams = { &strTok, nullptr, &numTok }; - ASSERT_EQUALS(15, CheckBufferOverrun::countSprintfLength("str%s%d%d", multipleParams)); - ASSERT_EQUALS(26, CheckBufferOverrun::countSprintfLength("str%-6s%08ld%08ld", multipleParams)); - } + Token floatTok2; + floatTok2.str("100.12345f"); + std::list floatAsParameter2(1, &floatTok2); + TODO_ASSERT_EQUALS(7, 3, CheckBufferOverrun::countSprintfLength("%2.2f", floatAsParameter2)); + TODO_ASSERT_EQUALS(7, 3, CheckBufferOverrun::countSprintfLength("%.2f", floatAsParameter)); + TODO_ASSERT_EQUALS(7, 5, CheckBufferOverrun::countSprintfLength("%4.2f", floatAsParameter)); + std::list multipleParams = { &strTok, nullptr, &numTok }; + ASSERT_EQUALS(15, CheckBufferOverrun::countSprintfLength("str%s%d%d", multipleParams)); + ASSERT_EQUALS(26, CheckBufferOverrun::countSprintfLength("str%-6s%08ld%08ld", multipleParams)); + } + */ void minsize_argvalue() { Settings settings; const char xmldata[] = "\n" @@ -3269,14 +3257,14 @@ private: " char s[10];\n" " mymemset(s, 0, '*');\n" "}", settings); - ASSERT_EQUALS("[test.cpp:3]: (warning) The size argument is given as a char constant.\n", errout.str()); + // TODO ASSERT_EQUALS("[test.cpp:3]: (warning) The size argument is given as a char constant.\n", errout.str()); // ticket #836 check("void f(void) {\n" " char a[10];\n" " mymemset(a+5, 0, 10);\n" "}", settings); - ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: a\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: a\n", "", errout.str()); // Ticket #909 check("void f(void) {\n" @@ -3304,7 +3292,7 @@ private: " char b[5][6];\n" " mymemset(b, 0, 5 * 6);\n" "}", settings); - ASSERT_EQUALS("", errout.str()); + // TODO ASSERT_EQUALS("", errout.str()); check("int main() {\n" " char b[5][6];\n" @@ -3325,7 +3313,7 @@ private: check("void f() {\n" " mymemset(\"abc\", 0, 20);\n" "}", settings); - ASSERT_EQUALS("[test.cpp:2]: (error) Buffer is accessed out of bounds.\n", errout.str()); + // TODO ASSERT_EQUALS("[test.cpp:2]: (error) Buffer is accessed out of bounds.\n", errout.str()); check("void f() {\n" " mymemset(temp, \"abc\", 4);\n" @@ -3333,10 +3321,10 @@ private: ASSERT_EQUALS("", errout.str()); check("void f() {\n" // #6816 - fp when array has known string value - " const char c[10] = \"c\";\n" + " char c[10] = \"c\";\n" " mymemset(c, 0, 10);\n" "}", settings); - ASSERT_EQUALS("", errout.str()); + // TODO ASSERT_EQUALS("", errout.str()); } void minsize_sizeof() { @@ -3346,7 +3334,7 @@ private: " \n" " false\n" " \n" - " \n" + " \n" " \n" " \n" " \n" @@ -3392,7 +3380,7 @@ private: " char buf[5];\n" " a(buf);" "}", settings); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1]: (error) Buffer is accessed out of bounds: buf\n", errout.str()); + // TODO CTU ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1]: (error) Buffer is accessed out of bounds: buf\n", errout.str()); } void minsize_strlen() { @@ -3445,7 +3433,7 @@ private: " char *str = new char[5];\n" " mysprintf(str, \"abcde\");\n" "}", settings); - ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", errout.str()); + // TODO ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", errout.str()); check("void f(int condition) {\n" " char str[5];\n" @@ -3464,20 +3452,20 @@ private: " struct Foo x;\n" " mysprintf(x.a, \"aa\");\n" "}", settings); - ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: x.a\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: x.a\n", "", errout.str()); // ticket #900 check("void f() {\n" " char *a = new char(30);\n" " mysprintf(a, \"a\");\n" "}", settings); - ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", errout.str()); + // TODO ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", errout.str()); check("void f(char value) {\n" " char *a = new char(value);\n" " mysprintf(a, \"a\");\n" "}", settings); - ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", errout.str()); + // TODO ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", errout.str()); // This is out of bounds if 'sizeof(ABC)' is 1 (No padding) check("struct Foo { char a[1]; };\n" @@ -3485,7 +3473,7 @@ private: " struct Foo *x = malloc(sizeof(Foo));\n" " mysprintf(x.a, \"aa\");\n" "}", settings); - TODO_ASSERT_EQUALS("error", "", errout.str()); + // TODO ASSERT_EQUALS("", errout.str()); check("struct Foo { char a[1]; };\n" "void f() {\n" @@ -3710,76 +3698,6 @@ private: "}"); } - void executionPaths1() { - check("void f(int a)\n" - "{\n" - " int buf[10];\n" - " int i = 5;\n" - " if (a == 1)\n" - " i = 1000;\n" - " buf[i] = 0;\n" - "}"); - ASSERT_EQUALS("[test.cpp:7]: (error) Array 'buf[10]' accessed at index 1000, which is out of bounds.\n", errout.str()); - - check("void f(int a)\n" - "{\n" - " int buf[10][5];\n" - " int i = 5;\n" - " if (a == 1)\n" - " i = 1000;\n" - " buf[i][0] = 0;\n" - "}"); - ASSERT_EQUALS("[test.cpp:7]: (error) Array 'buf[10][5]' index buf[1000][0] out of bounds.\n", errout.str()); - } - - void executionPaths2() { - check("void foo()\n" - "{\n" - " char a[64];\n" - " int sz = sizeof(a);\n" - " bar(&sz);\n" - " a[sz] = 0;\n" - "}"); - ASSERT_EQUALS("", errout.str()); - } - - void executionPaths3() { - check("void f(char *VLtext)\n" - "{\n" - " if ( x ) {\n" - " return VLtext[0];\n" - " } else {\n" - " int wordlen = ab();\n" - " VLtext[wordlen] = 0;\n" - " }\n" - "}"); - ASSERT_EQUALS("", errout.str()); - } - - void executionPaths5() { - // No false positive - check("class A {\n" - " void foo() {\n" - " int j = g();\n" - " arr[j]=0;\n" - " }\n" - "\n" - " int arr[2*BSize + 2];\n" - "};"); - ASSERT_EQUALS("", errout.str()); - } - - void executionPaths6() { // handling unknown type - const char code[] = "void f() {\n" - " u32 a[10];" - " u32 i = 0;\n" - " if (x) { i = 1000; }\n" - " a[i] = 0;\n" - "}"; - check(code); - ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[10]' accessed at index 1000, which is out of bounds.\n", errout.str()); - } - void insecureCmdLineArgs() { check("int main(int argc, char *argv[])\n" "{\n"