[analyzer] New checker for detecting usages of unsafe I/O functions

There are certain unsafe or deprecated (since C11) buffer handling
functions which should be avoided in safety critical code. They
could cause buffer overflows. A new checker,
'security.insecureAPI.DeprecatedOrUnsafeBufferHandling' warns for
every occurrence of such functions (unsafe or deprecated printf,
scanf family, and other buffer handling functions, which now have
a secure variant).

Patch by Dániel Kolozsvári!

Differential Revision: https://reviews.llvm.org/D35068


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@353698 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 195 insertions, 3 deletions

diff --git a/docs/analyzer/checkers.rst b/docs/analyzer/checkers.rst
index c6fcfe4d5e..62f7f7bfc8 100644
--- a/docs/analyzer/checkers.rst
+++ b/docs/analyzer/checkers.rst
@@ -566,6 +566,17 @@ security.insecureAPI.vfork (C)
    vfork(); // warn
  }
 
+security.insecureAPI.DeprecatedOrUnsafeBufferHandling (C) 
+""""""""""""""""""""""""""""""
+ Warn on occurrences of unsafe or deprecated buffer handling functions, which now have a secure variant: ``sprintf, vsprintf, scanf, wscanf, fscanf, fwscanf, vscanf, vwscanf, vfscanf, vfwscanf, sscanf, swscanf, vsscanf, vswscanf, swprintf, snprintf, vswprintf, vsnprintf, memcpy, memmove, strncpy, strncat, memset``
+
+.. code-block:: c
+ 
+ void test() {
+   char buf [5];
+   strncpy(buf, "a", 1); // warn
+ }
+
 .. _unix-checkers:
 
 unix
diff --git a/include/clang/StaticAnalyzer/Checkers/Checkers.td b/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 5603c0d54b..89ce3c62ff 100644
--- a/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -622,6 +622,13 @@ def UncheckedReturn : Checker<"UncheckedReturn">,
   Dependencies<[SecuritySyntaxChecker]>,
   Documentation<HasDocumentation>;
 
+def DeprecatedOrUnsafeBufferHandling :
+  Checker<"DeprecatedOrUnsafeBufferHandling">,
+  HelpText<"Warn on uses of unsecure or deprecated buffer manipulating "
+           "functions">,
+  Dependencies<[SecuritySyntaxChecker]>,
+  Documentation<HasDocumentation>;
+
 } // end "security.insecureAPI"
 
 let ParentPackage = Security in {
diff --git a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
index e297f8cdf2..05e25c4159 100644
--- a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -44,6 +44,7 @@ struct ChecksFilter {
   DefaultBool check_mktemp;
   DefaultBool check_mkstemp;
   DefaultBool check_strcpy;
+  DefaultBool check_DeprecatedOrUnsafeBufferHandling;
   DefaultBool check_rand;
   DefaultBool check_vfork;
   DefaultBool check_FloatLoopCounter;
@@ -57,6 +58,7 @@ struct ChecksFilter {
   CheckName checkName_mktemp;
   CheckName checkName_mkstemp;
   CheckName checkName_strcpy;
+  CheckName checkName_DeprecatedOrUnsafeBufferHandling;
   CheckName checkName_rand;
   CheckName checkName_vfork;
   CheckName checkName_FloatLoopCounter;
@@ -103,6 +105,8 @@ public:
   void checkCall_mkstemp(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD);
+  void checkDeprecatedOrUnsafeBufferHandling(const CallExpr *CE,
+                                             const FunctionDecl *FD);
   void checkCall_rand(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_random(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_vfork(const CallExpr *CE, const FunctionDecl *FD);
@@ -148,6 +152,14 @@ void WalkAST::VisitCallExpr(CallExpr *CE) {
     .Case("mkstemps", &WalkAST::checkCall_mkstemp)
     .Cases("strcpy", "__strcpy_chk", &WalkAST::checkCall_strcpy)
     .Cases("strcat", "__strcat_chk", &WalkAST::checkCall_strcat)
+    .Cases("sprintf", "vsprintf", "scanf", "wscanf", "fscanf", "fwscanf",
+           "vscanf", "vwscanf", "vfscanf", "vfwscanf",
+           &WalkAST::checkDeprecatedOrUnsafeBufferHandling)
+    .Cases("sscanf", "swscanf", "vsscanf", "vswscanf", "swprintf",
+           "snprintf", "vswprintf", "vsnprintf", "memcpy", "memmove",
+           &WalkAST::checkDeprecatedOrUnsafeBufferHandling)
+    .Cases("strncpy", "strncat", "memset",
+           &WalkAST::checkDeprecatedOrUnsafeBufferHandling)
     .Case("drand48", &WalkAST::checkCall_rand)
     .Case("erand48", &WalkAST::checkCall_rand)
     .Case("jrand48", &WalkAST::checkCall_rand)
@@ -552,7 +564,6 @@ void WalkAST::checkCall_mktemp(const CallExpr *CE, const FunctionDecl *FD) {
                      CELoc, CE->getCallee()->getSourceRange());
 }
 
-
 //===----------------------------------------------------------------------===//
 // Check: Use of 'mkstemp', 'mktemp', 'mkdtemp' should contain at least 6 X's.
 //===----------------------------------------------------------------------===//
@@ -641,6 +652,7 @@ void WalkAST::checkCall_mkstemp(const CallExpr *CE, const FunctionDecl *FD) {
 // CWE-119: Improper Restriction of Operations within
 // the Bounds of a Memory Buffer
 //===----------------------------------------------------------------------===//
+
 void WalkAST::checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD) {
   if (!filter.check_strcpy)
     return;
@@ -679,6 +691,7 @@ void WalkAST::checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD) {
 // CWE-119: Improper Restriction of Operations within
 // the Bounds of a Memory Buffer
 //===----------------------------------------------------------------------===//
+
 void WalkAST::checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD) {
   if (!filter.check_strcpy)
     return;
@@ -701,8 +714,88 @@ void WalkAST::checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD) {
 }
 
 //===----------------------------------------------------------------------===//
+// Check: Any use of 'sprintf', 'vsprintf', 'scanf', 'wscanf', 'fscanf',
+//        'fwscanf', 'vscanf', 'vwscanf', 'vfscanf', 'vfwscanf', 'sscanf',
+//        'swscanf', 'vsscanf', 'vswscanf', 'swprintf', 'snprintf', 'vswprintf',
+//        'vsnprintf', 'memcpy', 'memmove', 'strncpy', 'strncat', 'memset'
+//        is deprecated since C11.
+//
+//        Use of 'sprintf', 'vsprintf', 'scanf', 'wscanf','fscanf',
+//        'fwscanf', 'vscanf', 'vwscanf', 'vfscanf', 'vfwscanf', 'sscanf',
+//        'swscanf', 'vsscanf', 'vswscanf' without buffer limitations
+//        is insecure.
+//
+// CWE-119: Improper Restriction of Operations within
+// the Bounds of a Memory Buffer
+//===----------------------------------------------------------------------===//
+
+void WalkAST::checkDeprecatedOrUnsafeBufferHandling(const CallExpr *CE,
+                                                    const FunctionDecl *FD) {
+  if (!filter.check_DeprecatedOrUnsafeBufferHandling)
+    return;
+
+  if (!BR.getContext().getLangOpts().C11)
+    return;
+
+  // Issue a warning. ArgIndex == -1: Deprecated but not unsafe (has size
+  // restrictions).
+  enum { DEPR_ONLY = -1, UNKNOWN_CALL = -2 };
+  StringRef Name = FD->getIdentifier()->getName();
+  int ArgIndex =
+      llvm::StringSwitch<int>(Name)
+          .Cases("scanf", "wscanf", "vscanf", "vwscanf", 0)
+          .Cases("sprintf", "vsprintf", "fscanf", "fwscanf", "vfscanf",
+                 "vfwscanf", "sscanf", "swscanf", "vsscanf", "vswscanf", 1)
+          .Cases("swprintf", "snprintf", "vswprintf", "vsnprintf", "memcpy",
+                 "memmove", "memset", "strncpy", "strncat", DEPR_ONLY)
+          .Default(UNKNOWN_CALL);
+
+  assert(ArgIndex != UNKNOWN_CALL && "Unsupported function");
+  bool BoundsProvided = ArgIndex == DEPR_ONLY;
+
+  if (!BoundsProvided) {
+    // Currently we only handle (not wide) string literals. It is possible to do
+    // better, either by looking at references to const variables, or by doing
+    // real flow analysis.
+    auto FormatString =
+        dyn_cast<StringLiteral>(CE->getArg(ArgIndex)->IgnoreParenImpCasts());
+    if (FormatString &&
+        FormatString->getString().find("%s") == StringRef::npos &&
+        FormatString->getString().find("%[") == StringRef::npos)
+      BoundsProvided = true;
+  }
+
+  SmallString<128> Buf1;
+  SmallString<512> Buf2;
+  llvm::raw_svector_ostream Out1(Buf1);
+  llvm::raw_svector_ostream Out2(Buf2);
+
+  Out1 << "Potential insecure memory buffer bounds restriction in call '"
+       << Name << "'";
+  Out2 << "Call to function '" << Name
+       << "' is insecure as it does not provide ";
+
+  if (!BoundsProvided) {
+    Out2 << "bounding of the memory buffer or ";
+  }
+
+  Out2 << "security checks introduced "
+          "in the C11 standard. Replace with analogous functions that "
+          "support length arguments or provides boundary checks such as '"
+       << Name << "_s' in case of C11";
+
+  PathDiagnosticLocation CELoc =
+      PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
+  BR.EmitBasicReport(AC->getDecl(),
+                     filter.checkName_DeprecatedOrUnsafeBufferHandling,
+                     Out1.str(), "Security", Out2.str(), CELoc,
+                     CE->getCallee()->getSourceRange());
+}
+
+//===----------------------------------------------------------------------===//
 // Common check for str* functions with no bounds parameters.
 //===----------------------------------------------------------------------===//
+
 bool WalkAST::checkCall_strCommon(const CallExpr *CE, const FunctionDecl *FD) {
   const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>();
   if (!FPT)
@@ -936,5 +1029,4 @@ REGISTER_CHECKER(rand)
 REGISTER_CHECKER(vfork)
 REGISTER_CHECKER(FloatLoopCounter)
 REGISTER_CHECKER(UncheckedReturn)
-
-
+REGISTER_CHECKER(DeprecatedOrUnsafeBufferHandling)
diff --git a/test/Analysis/security-syntax-checks.m b/test/Analysis/security-syntax-checks.m
index 1fd00dffe4..68fc4b471a 100644
--- a/test/Analysis/security-syntax-checks.m
+++ b/test/Analysis/security-syntax-checks.m
@@ -13,6 +13,9 @@
 # define BUILTIN(f) f
 #endif /* USE_BUILTINS */
 
+#include "Inputs/system-header-simulator-for-valist.h"
+#include "Inputs/system-header-simulator-for-simple-stream.h"
+
 typedef typeof(sizeof(int)) size_t;
 
 
@@ -238,3 +241,82 @@ void test_mkstemp() {
   mkdtemp("XXXXXX");
 }
 
+
+//===----------------------------------------------------------------------===
+// deprecated or unsafe buffer handling
+//===----------------------------------------------------------------------===
+typedef int wchar_t;
+
+int sprintf(char *str, const char *format, ...);
+//int vsprintf (char *s, const char *format, va_list arg);
+int scanf(const char *format, ...);
+int wscanf(const wchar_t *format, ...);
+int fscanf(FILE *stream, const char *format, ...);
+int fwscanf(FILE *stream, const wchar_t *format, ...);
+int vscanf(const char *format, va_list arg);
+int vwscanf(const wchar_t *format, va_list arg);
+int vfscanf(FILE *stream, const char *format, va_list arg);
+int vfwscanf(FILE *stream, const wchar_t *format, va_list arg);
+int sscanf(const char *s, const char *format, ...);
+int swscanf(const wchar_t *ws, const wchar_t *format, ...);
+int vsscanf(const char *s, const char *format, va_list arg);
+int vswscanf(const wchar_t *ws, const wchar_t *format, va_list arg);
+int swprintf(wchar_t *ws, size_t len, const wchar_t *format, ...);
+int snprintf(char *s, size_t n, const char *format, ...);
+int vswprintf(wchar_t *ws, size_t len, const wchar_t *format, va_list arg);
+int vsnprintf(char *s, size_t n, const char *format, va_list arg);
+void *memcpy(void *destination, const void *source, size_t num);
+void *memmove(void *destination, const void *source, size_t num);
+char *strncpy(char *destination, const char *source, size_t num);
+char *strncat(char *destination, const char *source, size_t num);
+void *memset(void *ptr, int value, size_t num);
+
+void test_deprecated_or_unsafe_buffer_handling_1() {
+  char buf [5];
+  wchar_t wbuf [5];
+  int a;
+  FILE *file;
+  sprintf(buf, "a"); // expected-warning{{Call to function 'sprintf' is insecure}}
+  scanf("%d", &a); // expected-warning{{Call to function 'scanf' is insecure}}
+  scanf("%s", buf); // expected-warning{{Call to function 'scanf' is insecure}}
+  scanf("%4s", buf); // expected-warning{{Call to function 'scanf' is insecure}}
+  wscanf((const wchar_t*) L"%s", buf); // expected-warning{{Call to function 'wscanf' is insecure}}
+  fscanf(file, "%d", &a); // expected-warning{{Call to function 'fscanf' is insecure}}
+  fscanf(file, "%s", buf); // expected-warning{{Call to function 'fscanf' is insecure}}
+  fscanf(file, "%4s", buf); // expected-warning{{Call to function 'fscanf' is insecure}}
+  fwscanf(file, (const wchar_t*) L"%s", wbuf); // expected-warning{{Call to function 'fwscanf' is insecure}}
+  sscanf("5", "%d", &a); // expected-warning{{Call to function 'sscanf' is insecure}}
+  sscanf("5", "%s", buf); // expected-warning{{Call to function 'sscanf' is insecure}}
+  sscanf("5", "%4s", buf); // expected-warning{{Call to function 'sscanf' is insecure}}
+  swscanf(L"5", (const wchar_t*) L"%s", wbuf); // expected-warning{{Call to function 'swscanf' is insecure}}
+  swprintf(L"5", 1, (const wchar_t*) L"%s", wbuf); // expected-warning{{Call to function 'swprintf' is insecure}}
+  snprintf("5", 1, "%s", buf); // expected-warning{{Call to function 'snprintf' is insecure}}
+  memcpy(buf, wbuf, 1); // expected-warning{{Call to function 'memcpy' is insecure}}
+  memmove(buf, wbuf, 1); // expected-warning{{Call to function 'memmove' is insecure}}
+  strncpy(buf, "a", 1); // expected-warning{{Call to function 'strncpy' is insecure}}
+  strncat(buf, "a", 1); // expected-warning{{Call to function 'strncat' is insecure}}
+  memset(buf, 'a', 1); // expected-warning{{Call to function 'memset' is insecure}}
+}
+
+void test_deprecated_or_unsafe_buffer_handling_2(const char *format, ...) {
+  char buf [5];
+  FILE *file;
+  va_list args;
+  va_start(args, format);
+  vsprintf(buf, format, args); // expected-warning{{Call to function 'vsprintf' is insecure}}
+  vscanf(format, args); // expected-warning{{Call to function 'vscanf' is insecure}}
+  vfscanf(file, format, args); // expected-warning{{Call to function 'vfscanf' is insecure}}
+  vsscanf("a", format, args); // expected-warning{{Call to function 'vsscanf' is insecure}}
+  vsnprintf("a", 1, format, args); // expected-warning{{Call to function 'vsnprintf' is insecure}}
+}
+
+void test_deprecated_or_unsafe_buffer_handling_3(const wchar_t *format, ...) {
+  wchar_t wbuf [5];
+  FILE *file;
+  va_list args;
+  va_start(args, format);
+  vwscanf(format, args); // expected-warning{{Call to function 'vwscanf' is insecure}}
+  vfwscanf(file, format, args); // expected-warning{{Call to function 'vfwscanf' is insecure}}
+  vswscanf(L"a", format, args); // expected-warning{{Call to function 'vswscanf' is insecure}}
+  vswprintf(L"a", 1, format, args); // expected-warning{{Call to function 'vswprintf' is insecure}}
+}