New check: Pointer overflow (null pointer subtracted)

lib/checknullpointer.cpp

@@ -493,3 +493,50 @@ void CheckNullPointer::nullPointerError(const Token *tok, const std::string &var
     const std::string errmsg(ValueFlow::eitherTheConditionIsRedundant(nullCheck) + " or there is possible null pointer dereference: " + varname + ".");
     reportError(callstack, Severity::warning, "nullPointerRedundantCheck", errmsg, CWE476, inconclusive);
 }
+
+void CheckNullPointer::arithmetic()
+{
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
+    const std::size_t functions = symbolDatabase->functionScopes.size();
+    for (std::size_t i = 0; i < functions; ++i) {
+        const Scope * scope = symbolDatabase->functionScopes[i];
+        for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
+            if (!tok->astOperand2() || tok->str() != "-")
+                continue;
+            // pointer subtraction
+            if (!tok->valueType() || !tok->valueType()->pointer)
+                continue;
+            // Can LHS be NULL?
+            const ValueFlow::Value *value = tok->astOperand1()->getValue(0);
+            if (!value)
+                continue;
+            if (!_settings->inconclusive && value->inconclusive)
+                continue;
+            if (value->condition && !_settings->isEnabled("warning"))
+                continue;
+            arithmeticError(tok,value);
+        }
+    }
+}
+
+void CheckNullPointer::arithmeticError(const Token *tok, const ValueFlow::Value *value)
+{
+    std::string errmsg;
+    if (value && value->condition)
+        errmsg = ValueFlow::eitherTheConditionIsRedundant(value->condition) + " or there is overflow in pointer subtraction.";
+    else
+        errmsg = "Overflow in pointer arithmetic, NULL pointer is subtracted.";
+
+    std::list<const Token*> callstack;
+    callstack.push_back(tok);
+    if (value && value->condition)
+        callstack.push_back(value->condition);
+
+    reportError(callstack,
+                (value && value->condition) ? Severity::warning : Severity::error,
+                (value && value->condition) ? "nullPointerArithmeticRedundantCheck" : "nullPointerArithmetic",
+                errmsg,
+                CWE(0), // unknown - pointer overflow
+                value && value->inconclusive);
+}
+

lib/checknullpointer.h

@@ -47,6 +47,7 @@ public:
     void runChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
         CheckNullPointer checkNullPointer(tokenizer, settings, errorLogger);
         checkNullPointer.nullPointer();
+        checkNullPointer.arithmetic();
     }
 
     /** @brief Run checks against the simplified token list */
@@ -96,6 +97,7 @@ private:
         c.nullPointerError(nullptr);
         c.nullPointerError(nullptr, "pointer", false, true, true);
         c.nullPointerError(nullptr, "pointer", nullptr, false);
+        c.arithmeticError(nullptr, nullptr);
     }
 
     /** Name of check */
@@ -106,7 +108,8 @@ private:
     /** class info in WIKI format. Used by --doc */
     std::string classInfo() const {
         return "Null pointers\n"
-               "- null pointer dereferencing\n";
+               "- null pointer dereferencing\n"
+               "- undefined null pointer arithmetic\n";
     }
 
     /**
@@ -120,6 +123,10 @@ private:
      * Dereferencing a pointer and then checking if it's NULL..
      */
     void nullPointerByDeRefAndChec();
+
+    /** undefined null pointer arithmetic */
+    void arithmetic();
+    void arithmeticError(const Token *tok, const ValueFlow::Value *value);
 };
 /// @}
 //---------------------------------------------------------------------------

test/testnullpointer.cpp

@@ -96,6 +96,7 @@ private:
         TEST_CASE(functioncallDefaultArguments);
         TEST_CASE(nullpointer_internal_error); // #5080
         TEST_CASE(ticket6505);
+        TEST_CASE(subtract);
     }
 
     void check(const char code[], bool inconclusive = false, const char filename[] = "test.cpp") {
@@ -111,12 +112,12 @@ private:
             return;
 
         // Check for redundant code..
-        CheckNullPointer checkNullPointer(&tokenizer, &settings, this);
-        checkNullPointer.nullPointer();
+        CheckNullPointer checkNullPointer;
+        checkNullPointer.runChecks(&tokenizer, &settings, this);
 
         tokenizer.simplifyTokenList2();
 
-        checkNullPointer.nullConstantDereference();
+        checkNullPointer.runSimplifiedChecks(&tokenizer, &settings, this);
     }
 
 
@@ -2515,6 +2516,20 @@ private:
               "}\n", true, "test.c");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void subtract() {
+        check("void foo(char *s) {\n"
+              "  p = s - 20;\n"
+              "}\n"
+              "void bar() { foo(0); }\n");
+        ASSERT_EQUALS("[test.cpp:2]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.\n", errout.str());
+
+        check("void foo(char *s) {\n"
+              "  if (!s) {}\n"
+              "  p = s - 20;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition '!s' is redundant or there is overflow in pointer subtraction.\n", errout.str());
+    }
 };
 
 REGISTER_TEST(TestNullPointer)