CheckBufferOverrun: Add CTU analysis

lib/checkbufferoverrun.cpp

@@ -636,3 +636,106 @@ void CheckBufferOverrun::bufferNotZeroTerminatedError(const Token *tok, const st
 }
 
 
+
+//---------------------------------------------------------------------------
+// CTU..
+//---------------------------------------------------------------------------
+
+std::string CheckBufferOverrun::MyFileInfo::toString() const OVERRIDE
+{
+    return CTU::toString(unsafeUsage);
+}
+
+bool CheckBufferOverrun::isCtuUnsafeBufferUsage(const Check *check, const Token *argtok, MathLib::bigint *offset)
+{
+    const CheckBufferOverrun *c = dynamic_cast<const CheckBufferOverrun *>(check);
+    if (!c)
+        return false;
+    if (!argtok->valueType())
+        return false;
+    if (!Token::Match(argtok, "%name% [") || argtok->astParent() != argtok->next() || Token::simpleMatch(argtok->linkAt(1), "] ["))
+        return false;
+    if (!argtok->next()->astOperand2())
+        return false;
+    if (!argtok->next()->astOperand2()->hasKnownIntValue())
+        return false;
+    if (!offset)
+        return false;
+    *offset = argtok->next()->astOperand2()->getKnownIntValue() * argtok->valueType()->typeSize(*c->mSettings);
+    return true;
+}
+
+/** @brief Parse current TU and extract file info */
+Check::FileInfo *CheckBufferOverrun::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const OVERRIDE
+{
+    const std::list<CTU::FileInfo::UnsafeUsage> &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, this, isCtuUnsafeBufferUsage);
+    if (unsafeUsage.empty())
+        return nullptr;
+
+    MyFileInfo *fileInfo = new MyFileInfo;
+    fileInfo->unsafeUsage = unsafeUsage;
+    return fileInfo;
+}
+
+Check::FileInfo * CheckBufferOverrun::loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const OVERRIDE
+{
+    const std::list<CTU::FileInfo::UnsafeUsage> &unsafeUsage = CTU::loadUnsafeUsageListFromXml(xmlElement);
+    if (unsafeUsage.empty())
+        return nullptr;
+
+    MyFileInfo *fileInfo = new MyFileInfo;
+    fileInfo->unsafeUsage = unsafeUsage;
+    return fileInfo;
+}
+
+/** @brief Analyse all file infos for all TU */
+bool CheckBufferOverrun::analyseWholeProgram(const CTU::FileInfo *ctu, const std::list<Check::FileInfo*> &fileInfo, const Settings& settings, ErrorLogger &errorLogger) OVERRIDE {
+    if (!ctu)
+        return false;
+    bool foundErrors = false;
+    (void)settings; // This argument is unused
+
+    const std::map<std::string, std::list<const CTU::FileInfo::CallBase *>> callsMap = ctu->getCallsMap();
+
+    for (Check::FileInfo *fi1 : fileInfo)
+    {
+        const MyFileInfo *fi = dynamic_cast<MyFileInfo*>(fi1);
+        if (!fi)
+            continue;
+        for (const CTU::FileInfo::UnsafeUsage &unsafeUsage : fi->unsafeUsage) {
+            const CTU::FileInfo::FunctionCall *functionCall = nullptr;
+
+            const std::list<ErrorLogger::ErrorMessage::FileLocation> &locationList =
+            ctu->getErrorPath(CTU::FileInfo::InvalidValueType::bufferOverflow,
+                              unsafeUsage,
+                              callsMap,
+                              "Using argument ARG",
+                              &functionCall,
+                              false);
+            if (locationList.empty())
+                continue;
+
+            if (unsafeUsage.value > 0) {
+                const ErrorLogger::ErrorMessage errmsg(locationList,
+                                                       emptyString,
+                                                       Severity::error,
+                                                       "Buffer access out of bounds; '" + unsafeUsage.myArgumentName + "' buffer size is " + MathLib::toString(functionCall->callArgValue) + " and it is accessed at offset " + MathLib::toString(unsafeUsage.value) + ".",
+                                                       "ctubufferoverrun",
+                                                       CWE788, false);
+                errorLogger.reportErr(errmsg);
+            } else {
+                const ErrorLogger::ErrorMessage errmsg(locationList,
+                                                       emptyString,
+                                                       Severity::error,
+                                                       "Buffer access out of bounds; buffer '" + unsafeUsage.myArgumentName + "' is accessed at offset " + MathLib::toString(unsafeUsage.value) + ".",
+                                                       "ctubufferunderrun",
+                                                       CWE786, false);
+                errorLogger.reportErr(errmsg);
+            }
+
+            foundErrors = true;
+        }
+    }
+    return foundErrors;
+}
+

lib/checkbufferoverrun.h

@@ -24,6 +24,7 @@
 
 #include "check.h"
 #include "config.h"
+#include "ctu.h"
 #include "errorlogger.h"
 #include "mathlib.h"
 #include "tokenize.h"
@@ -76,6 +77,13 @@ public:
         c.bufferOverflowError(nullptr, nullptr);
     }
 
+    /** @brief Parse current TU and extract file info */
+    Check::FileInfo *getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const OVERRIDE;
+
+    /** @brief Analyse all file infos for all TU */
+    bool analyseWholeProgram(const CTU::FileInfo *ctu, const std::list<Check::FileInfo*> &fileInfo, const Settings& settings, ErrorLogger &errorLogger) OVERRIDE;
+
+
 private:
 
     void arrayIndex();
@@ -94,6 +102,23 @@ private:
 
     ValueFlow::Value getBufferSize(const Token *bufTok) const;
 
+    // CTU
+
+    /** data for multifile checking */
+    class MyFileInfo : public Check::FileInfo {
+    public:
+        /** function arguments that data are unconditionally read */
+        std::list<CTU::FileInfo::UnsafeUsage> unsafeUsage;
+
+        /** Convert MyFileInfo data into xml string */
+        std::string toString() const OVERRIDE;
+    };
+
+    static bool isCtuUnsafeBufferUsage(const Check *check, const Token *argtok, MathLib::bigint *value);
+
+    Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const OVERRIDE;
+
+
     static std::string myName() {
         return "Bounds checking";
     }

lib/checknullpointer.cpp

@@ -594,8 +594,9 @@ std::string CheckNullPointer::MyFileInfo::toString() const
     return CTU::toString(unsafeUsage);
 }
 
-static bool isUnsafeUsage(const Check *check, const Token *vartok)
+static bool isUnsafeUsage(const Check *check, const Token *vartok, MathLib::bigint *value)
 {
+    (void)value;
     const CheckNullPointer *checkNullPointer = dynamic_cast<const CheckNullPointer *>(check);
     bool unknown = false;
     return checkNullPointer && checkNullPointer->isPointerDeRef(vartok, unknown);

lib/checkuninitvar.cpp

@@ -1342,8 +1342,9 @@ Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer *tokenizer, const S
     return checker.getFileInfo();
 }
 
-static bool isVariableUsage(const Check *check, const Token *vartok)
+static bool isVariableUsage(const Check *check, const Token *vartok, MathLib::bigint *value)
 {
+    (void)value;
     const CheckUninitVar *c = dynamic_cast<const CheckUninitVar *>(check);
     return c && c->isVariableUsage(vartok, true, CheckUninitVar::Alloc::ARRAY);
 }

lib/ctu.cpp

@@ -37,6 +37,7 @@ static const char ATTR_INFO[] = "info";
 static const char ATTR_MY_ID[] = "my-id";
 static const char ATTR_MY_ARGNR[] = "my-argnr";
 static const char ATTR_MY_ARGNAME[] = "my-argname";
+static const char ATTR_VALUE[] = "value";
 
 int CTU::maxCtuDepth = 2;
 
@@ -123,6 +124,7 @@ std::string CTU::FileInfo::UnsafeUsage::toString() const
         << " " << ATTR_MY_ARGNAME << "=\"" << myArgumentName << '\"'
         << " " << ATTR_LOC_FILENAME << "=\"" << location.fileName << '\"'
         << " " << ATTR_LOC_LINENR << "=\"" << location.linenr << '\"'
+        << " " << ATTR_VALUE << "=\"" << value << "\""
         << "/>\n";
     return out.str();
 }
@@ -246,6 +248,8 @@ std::list<CTU::FileInfo::UnsafeUsage> CTU::loadUnsafeUsageListFromXml(const tiny
         unsafeUsage.myArgumentName = readAttrString(e, ATTR_MY_ARGNAME, &error);
         unsafeUsage.location.fileName = readAttrString(e, ATTR_LOC_FILENAME, &error);
         unsafeUsage.location.linenr = readAttrInt(e, ATTR_LOC_LINENR, &error);
+        unsafeUsage.value = readAttrInt(e, ATTR_VALUE, &error);
+
         if (!error)
             ret.push_back(unsafeUsage);
     }
@@ -306,10 +310,10 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer *tokenizer)
                 if (!argtok)
                     continue;
                 for (const ValueFlow::Value &value : argtok->values()) {
-                    if (!value.isIntValue() || value.intvalue != 0 || value.isInconclusive())
+                    if ((!value.isIntValue() || value.intvalue != 0 || value.isInconclusive()) && !value.isBufferSizeValue())
                         continue;
                     FileInfo::FunctionCall functionCall;
-                    functionCall.callValueType = ValueFlow::Value::INT;
+                    functionCall.callValueType = value.valueType;
                     functionCall.callId = getFunctionId(tokenizer, tok->astOperand1()->function());
                     functionCall.callFunctionName = tok->astOperand1()->expressionString();
                     functionCall.location.fileName = tokenizer->list.file(tok);
@@ -369,18 +373,19 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer *tokenizer)
     return fileInfo;
 }
 
-static bool isUnsafeFunction(const Tokenizer *tokenizer, const Settings *settings, const Scope *scope, int argnr, const Token **tok, const Check *check, bool (*isUnsafeUsage)(const Check *check, const Token *argtok))
+static std::list<std::pair<const Token *, MathLib::bigint>> getUnsafeFunction(const Tokenizer *tokenizer, const Settings *settings, const Scope *scope, int argnr, const Check *check, bool (*isUnsafeUsage)(const Check *check, const Token *argtok, MathLib::bigint *value))
 {
+    std::list<std::pair<const Token *, MathLib::bigint>> ret;
     const Variable * const argvar = scope->function->getArgumentVar(argnr);
     if (!argvar->isPointer())
-        return false;
+        return ret;
     for (const Token *tok2 = scope->bodyStart; tok2 != scope->bodyEnd; tok2 = tok2->next()) {
         if (Token::Match(tok2, ")|else {")) {
             tok2 = tok2->linkAt(1);
             if (Token::findmatch(tok2->link(), "return|throw", tok2))
-                return false;
+                return ret;
             if (isVariableChanged(tok2->link(), tok2, argvar->declarationId(), false, settings, tokenizer->isCPP()))
-                return false;
+                return ret;
         }
         if (Token::Match(tok2, "%oror%|&&|?")) {
             tok2 = tok2->findExpressionStartEndTokens().second;
@@ -388,15 +393,16 @@ static bool isUnsafeFunction(const Tokenizer *tokenizer, const Settings *setting
         }
         if (tok2->variable() != argvar)
             continue;
-        if (!isUnsafeUsage(check, tok2))
+        MathLib::bigint value = 0;
-            return false;
+        if (!isUnsafeUsage(check, tok2, &value))
-        *tok = tok2;
+            return ret; // TODO: Is this a read? then continue..
-        return true;
+        ret.emplace_back(tok2, value);
+        return ret;
     }
-    return false;
+    return ret;
 }
 
-std::list<CTU::FileInfo::UnsafeUsage> CTU::getUnsafeUsage(const Tokenizer *tokenizer, const Settings *settings, const Check *check, bool (*isUnsafeUsage)(const Check *check, const Token *argtok))
+std::list<CTU::FileInfo::UnsafeUsage> CTU::getUnsafeUsage(const Tokenizer *tokenizer, const Settings *settings, const Check *check, bool (*isUnsafeUsage)(const Check *check, const Token *argtok, MathLib::bigint *_value))
 {
     std::list<CTU::FileInfo::UnsafeUsage> unsafeUsage;
 
@@ -410,9 +416,11 @@ std::list<CTU::FileInfo::UnsafeUsage> CTU::getUnsafeUsage(const Tokenizer *token
 
         // "Unsafe" functions unconditionally reads data before it is written..
         for (int argnr = 0; argnr < function->argCount(); ++argnr) {
-            const Token *tok;
+            for (const std::pair<const Token *, MathLib::bigint> &v : getUnsafeFunction(tokenizer, settings, &scope, argnr, check, isUnsafeUsage)) {
-            if (isUnsafeFunction(tokenizer, settings, &scope, argnr, &tok, check, isUnsafeUsage))
+                const Token *tok = v.first;
-                unsafeUsage.push_back(CTU::FileInfo::UnsafeUsage(CTU::getFunctionId(tokenizer, function), argnr+1, tok->str(), CTU::FileInfo::Location(tokenizer,tok)));
+                MathLib::bigint value = v.second;
+                unsafeUsage.emplace_back(CTU::getFunctionId(tokenizer, function), argnr+1, tok->str(), CTU::FileInfo::Location(tokenizer,tok), value);
+            }
         }
     }
 
@@ -421,6 +429,7 @@ std::list<CTU::FileInfo::UnsafeUsage> CTU::getUnsafeUsage(const Tokenizer *token
 
 static bool findPath(const std::string &callId,
                      unsigned int callArgNr,
+                     MathLib::bigint unsafeValue,
                      CTU::FileInfo::InvalidValueType invalidValue,
                      const std::map<std::string, std::list<const CTU::FileInfo::CallBase *>> &callsMap,
                      const CTU::FileInfo::CallBase *path[10],
@@ -451,6 +460,12 @@ static bool findPath(const std::string &callId,
                 if (functionCall->callValueType != ValueFlow::Value::UNINIT)
                     continue;
                 break;
+            case CTU::FileInfo::InvalidValueType::bufferOverflow:
+                if (functionCall->callValueType != ValueFlow::Value::BUFFER_SIZE)
+                    continue;
+                if (unsafeValue < 0 || unsafeValue >= functionCall->callArgValue)
+                    break;
+                continue;
             };
             path[index] = functionCall;
             return true;
@@ -460,7 +475,7 @@ static bool findPath(const std::string &callId,
         if (!nestedCall)
             continue;
 
-        if (findPath(nestedCall->myId, nestedCall->myArgNr, invalidValue, callsMap, path, index + 1, warning)) {
+        if (findPath(nestedCall->myId, nestedCall->myArgNr, unsafeValue, invalidValue, callsMap, path, index + 1, warning)) {
             path[index] = nestedCall;
             return true;
         }
@@ -480,7 +495,7 @@ std::list<ErrorLogger::ErrorMessage::FileLocation> CTU::FileInfo::getErrorPath(I
 
     const CTU::FileInfo::CallBase *path[10] = {0};
 
-    if (!findPath(unsafeUsage.myId, unsafeUsage.myArgNr, invalidValue, callsMap, path, 0, warning))
+    if (!findPath(unsafeUsage.myId, unsafeUsage.myArgNr, unsafeUsage.value, invalidValue, callsMap, path, 0, warning))
         return locationList;
 
     const std::string value1 = (invalidValue == InvalidValueType::null) ? "null" : "uninitialized";

lib/ctu.h

@@ -33,7 +33,7 @@
 namespace CTU {
     class CPPCHECKLIB FileInfo : public Check::FileInfo {
     public:
-        enum InvalidValueType { null, uninit };
+        enum InvalidValueType { null, uninit, bufferOverflow };
 
         std::string toString() const OVERRIDE;
 
@@ -47,11 +47,12 @@ namespace CTU {
 
         struct UnsafeUsage {
             UnsafeUsage() = default;
-            UnsafeUsage(const std::string &myId, unsigned int myArgNr, const std::string &myArgumentName, const Location &location) : myId(myId), myArgNr(myArgNr), myArgumentName(myArgumentName), location(location) {}
+            UnsafeUsage(const std::string &myId, unsigned int myArgNr, const std::string &myArgumentName, const Location &location, MathLib::bigint value) : myId(myId), myArgNr(myArgNr), myArgumentName(myArgumentName), location(location), value(value) {}
             std::string myId;
             unsigned int myArgNr;
             std::string myArgumentName;
             Location location;
+            MathLib::bigint value;
             std::string toString() const;
         };
 
@@ -126,7 +127,7 @@ namespace CTU {
     /** @brief Parse current TU and extract file info */
     CPPCHECKLIB FileInfo *getFileInfo(const Tokenizer *tokenizer);
 
-    CPPCHECKLIB std::list<FileInfo::UnsafeUsage> getUnsafeUsage(const Tokenizer *tokenizer, const Settings *settings, const Check *check, bool (*isUnsafeUsage)(const Check *check, const Token *argtok));
+    CPPCHECKLIB std::list<FileInfo::UnsafeUsage> getUnsafeUsage(const Tokenizer *tokenizer, const Settings *settings, const Check *check, bool (*isUnsafeUsage)(const Check *check, const Token *argtok, MathLib::bigint *value));
 
     CPPCHECKLIB std::list<FileInfo::UnsafeUsage> loadUnsafeUsageListFromXml(const tinyxml2::XMLElement *xmlElement);
 }

test/testbufferoverrun.cpp

@@ -89,7 +89,7 @@ private:
         // TODO string TEST_CASE(array_index_4);
         TEST_CASE(array_index_6);
         TEST_CASE(array_index_7);
-        // TODO CTU 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);
@@ -245,6 +245,8 @@ private:
         TEST_CASE(negativeArraySize);
 
         // TODO TEST_CASE(pointerAddition1);
+
+        TEST_CASE(ctu_1);
     }
 
 
@@ -4114,6 +4116,53 @@ private:
               "\n");
         ASSERT_EQUALS("error", errout.str());
     }
+
+    void ctu(const char code[]) {
+        // Clear the error buffer..
+        errout.str("");
+
+        // Tokenize..
+        Tokenizer tokenizer(&settings0, this);
+        std::istringstream istr(code);
+        tokenizer.tokenize(istr, "test.cpp");
+
+        CTU::FileInfo *ctu = CTU::getFileInfo(&tokenizer);
+
+        // Check code..
+        std::list<Check::FileInfo*> fileInfo;
+        CheckBufferOverrun check(&tokenizer, &settings0, this);
+        fileInfo.push_back(check.getFileInfo(&tokenizer, &settings0));
+        check.analyseWholeProgram(ctu, fileInfo, settings0, *this);
+        while (!fileInfo.empty()) {
+            delete fileInfo.back();
+            fileInfo.pop_back();
+        }
+        delete ctu;
+    }
+
+    void ctu_1() {
+        ctu("void dostuff(const char *p) {\n"
+            "  p[-3] = 0;\n"
+            "}\n"
+            "\n"
+            "int main() {\n"
+            "  char *s = malloc(4);\n"
+            "  dostuff(s);\n"
+            "  return 0;\n"
+            "}");
+        ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:7] -> [test.cpp:2]: (error) Buffer access out of bounds; buffer 'p' is accessed at offset -3.\n", errout.str());
+
+        ctu("void dostuff(const char *p) {\n"
+            "  p[4] = 0;\n"
+            "}\n"
+            "\n"
+            "int main() {\n"
+            "  char *s = malloc(4);\n"
+            "  dostuff(s);\n"
+            "  return 0;\n"
+            "}");
+        ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:7] -> [test.cpp:2]: (error) Buffer access out of bounds; 'p' buffer size is 4 and it is accessed at offset 4.\n", errout.str());
+    }
 };
 
 REGISTER_TEST(TestBufferOverrun)