Improved checking of scanf format strings

lib/checkio.cpp

@@ -25,6 +25,7 @@
 #include "symboldatabase.h"
 
 #include <cctype>
+#include <cstdlib>
 
 //---------------------------------------------------------------------------
 
@@ -443,6 +444,7 @@ void CheckIO::checkWrongPrintfScanfArguments()
                 percent = false;
 
                 bool _continue = false;
+                std::string width;
                 while (i != formatString.end() && *i != ']' && !std::isalpha(*i)) {
                     if (*i == '*') {
                         if (scan)
@@ -452,7 +454,8 @@ void CheckIO::checkWrongPrintfScanfArguments()
                             if (argListTok)
                                 argListTok = argListTok->nextArgument();
                         }
-                    }
+                    } else if (std::isdigit(*i))
+                        width += *i;
                     ++i;
                 }
                 if (i == formatString.end())
@@ -473,6 +476,16 @@ void CheckIO::checkWrongPrintfScanfArguments()
                         if (scan && varTypeTok) {
                             if ((!variableInfo->isPointer() && !variableInfo->isArray()) || varTypeTok->strAt(-1) == "const")
                                 invalidScanfArgTypeError(tok, tok->str(), numFormat);
+
+                            if (*i == 's' && variableInfo && isKnownType(variableInfo, varTypeTok) && variableInfo->isArray() && (variableInfo->dimensions().size() == 1)) {
+                                if (width.empty())
+                                    missingScanfFormatWidthError(tok, tok->str(), numFormat, variableInfo);
+                                else {
+                                    int numWidth = std::atoi(width.c_str());
+                                    if (numWidth  != (variableInfo->dimension(0) - 1))
+                                        invalidScanfFormatWidthError(tok, tok->str(), numFormat, numWidth, variableInfo);
+                                }
+                            }
                         } else if (!scan) {
                             switch (*i) {
                             case 's':
@@ -599,3 +612,35 @@ void CheckIO::invalidPrintfArgTypeError_float(const Token* tok, unsigned int num
     errmsg << "%" << c << " in format string (no. " << numFormat << ") requires a floating point number given in the argument list";
     reportError(tok, Severity::warning, "invalidPrintfArgType_float", errmsg.str());
 }
+void CheckIO::missingScanfFormatWidthError(const Token* tok, const std::string &functionName, unsigned int numFormat, const Variable *var)
+{
+    std::ostringstream errmsg;
+    errmsg << functionName << " %s in format string (no. " << numFormat << ") does not specify a width";
+    if (var)
+        errmsg << ", use %" << (var->dimension(0) - 1) << "s to prevent overflowing destination: " << var->name() << "[" << var->dimension(0) << "]";
+    reportError(tok, Severity::warning, "missingScanfFormatWidth", errmsg.str());
+}
+void CheckIO::invalidScanfFormatWidthError(const Token* tok, const std::string &functionName, unsigned int numFormat, int width, const Variable *var)
+{
+    std::ostringstream errmsg;
+    Severity::SeverityType severity = Severity::warning;
+    bool inconclusive = false;
+
+    if (var) {
+        if (var->dimension(0) > width) {
+            if (!_settings->inconclusive)
+                return;
+            inconclusive = true;
+            errmsg << functionName << " width " << width << " in format string (no. " << numFormat << ") is smaller than destination"
+                   << ": " << var->name() << "[" << var->dimension(0) << "]";
+        } else {
+            errmsg << functionName << " width " << width << " in format string (no. " << numFormat << ") is larger than destination"
+                   << ", use %" << (var->dimension(0) - 1) << "s to prevent overflowing destination: " << var->name() << "[" << var->dimension(0) << "]";
+            severity = Severity::error;
+        }
+
+    } else
+        errmsg << functionName << " width " << width << " in format string (no. " << numFormat << ") doesn't match destination";
+
+    reportError(tok, severity, "invalidScanfFormatWidth", errmsg.str(), inconclusive);
+}

lib/checkio.h

@@ -24,10 +24,11 @@
 #include "check.h"
 #include "config.h"
 
+class Variable;
+
 /// @addtogroup Checks
 /// @{
 
-
 /** @brief %Check input output operations. */
 class CPPCHECKLIB CheckIO : public Check {
 public:
@@ -87,6 +88,8 @@ private:
     void invalidPrintfArgTypeError_p(const Token* tok, unsigned int numFormat);
     void invalidPrintfArgTypeError_int(const Token* tok, unsigned int numFormat, char c);
     void invalidPrintfArgTypeError_float(const Token* tok, unsigned int numFormat, char c);
+    void missingScanfFormatWidthError(const Token* tok, const std::string &functionName, unsigned int numFormat, const Variable *var);
+    void invalidScanfFormatWidthError(const Token* tok, const std::string &functionName, unsigned int numFormat, int width, const Variable *var);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         CheckIO c(0, settings, errorLogger);
@@ -105,6 +108,8 @@ private:
         c.invalidPrintfArgTypeError_p(0, 1);
         c.invalidPrintfArgTypeError_int(0, 1, 'u');
         c.invalidPrintfArgTypeError_float(0, 1, 'f');
+        c.missingScanfFormatWidthError(0, "scanf", 1, NULL);
+        c.invalidScanfFormatWidthError(0, "scanf", 10, 5, NULL);
     }
 
     std::string myName() const {

test/testio.cpp

@@ -48,12 +48,13 @@ private:
         TEST_CASE(testPrintfArgument);
     }
 
-    void check(const char code[]) {
+    void check(const char code[], bool inconclusive = false) {
         // Clear the error buffer..
         errout.str("");
 
         Settings settings;
         settings.addEnabled("style");
+        settings.inconclusive = inconclusive;
 
         // Tokenize..
         Tokenizer tokenizer(&settings, this);
@@ -443,6 +444,31 @@ private:
                       "[test.cpp:3]: (error) scanf format string has 2 parameters but only 1 are given\n"
                       "[test.cpp:4]: (error) sscanf format string has 2 parameters but only 1 are given\n"
                       "[test.cpp:5]: (error) scanf format string has 2 parameters but only 1 are given\n", errout.str());
+
+        check("void foo() {\n"
+              "    char input[10];\n"
+              "    char output[5];\n"
+              "    sscanf(input, \"%s\", output);\n"
+              "    sscanf(input, \"%3s\", output);\n"
+              "    sscanf(input, \"%4s\", output);\n"
+              "    sscanf(input, \"%5s\", output);\n"
+              "}", false);
+        ASSERT_EQUALS("[test.cpp:4]: (warning) sscanf %s in format string (no. 1) does not specify a width, use %4s to prevent overflowing destination: output[5]\n"
+                      "[test.cpp:7]: (error) sscanf width 5 in format string (no. 1) is larger than destination, use %4s to prevent overflowing destination: output[5]\n"
+                      "[test.cpp:4]: (warning) scanf without field width limits can crash with huge input data\n", errout.str());
+
+        check("void foo() {\n"
+              "    char input[10];\n"
+              "    char output[5];\n"
+              "    sscanf(input, \"%s\", output);\n"
+              "    sscanf(input, \"%3s\", output);\n"
+              "    sscanf(input, \"%4s\", output);\n"
+              "    sscanf(input, \"%5s\", output);\n"
+              "}", true);
+        ASSERT_EQUALS("[test.cpp:4]: (warning) sscanf %s in format string (no. 1) does not specify a width, use %4s to prevent overflowing destination: output[5]\n"
+                      "[test.cpp:5]: (warning, inconclusive) sscanf width 3 in format string (no. 1) is smaller than destination: output[5]\n"
+                      "[test.cpp:7]: (error) sscanf width 5 in format string (no. 1) is larger than destination, use %4s to prevent overflowing destination: output[5]\n"
+                      "[test.cpp:4]: (warning) scanf without field width limits can crash with huge input data\n", errout.str());
     }
 
     void testPrintfArgument() {