Add security syntax checker for strcpy() which causes the Static Analyzer to generate a warning any time the strcpy() function is used with a note suggesting to use a function which provides bounded buffers.

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

2 files changed, 91 insertions, 0 deletions

diff --git a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
index 8b1756937e..61d3e4c652 100644
--- a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -66,6 +66,7 @@ public:
   void CheckCall_gets(const CallExpr *CE, const FunctionDecl *FD);
   void CheckCall_getpw(const CallExpr *CE, const FunctionDecl *FD);
   void CheckCall_mktemp(const CallExpr *CE, const FunctionDecl *FD);
+  void CheckCall_strcpy(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 CheckUncheckedReturnValue(CallExpr *CE);
@@ -98,6 +99,7 @@ void WalkAST::VisitCallExpr(CallExpr *CE) {
     CheckCall_gets(CE, FD);
     CheckCall_getpw(CE, FD);
     CheckCall_mktemp(CE, FD);
+    CheckCall_strcpy(CE, FD);
     if (CheckRand) {
       CheckCall_rand(CE, FD);
       CheckCall_random(CE, FD);
@@ -349,6 +351,58 @@ void WalkAST::CheckCall_mktemp(const CallExpr *CE, const FunctionDecl *FD) {
 }
 
 //===----------------------------------------------------------------------===//
+// Check: Any use of 'strcpy' is insecure.
+//
+// CWE-119: Improper Restriction of Operations within 
+// the Bounds of a Memory Buffer 
+//===----------------------------------------------------------------------===//
+void WalkAST::CheckCall_strcpy(const CallExpr *CE, const FunctionDecl *FD) {
+  IdentifierInfo *II = FD->getIdentifier();
+  if (!II)   // if no identifier, not a simple C function
+    return;
+  llvm::StringRef Name = II->getName();
+  if (Name.startswith("__builtin_"))
+    Name = Name.substr(10);
+
+  if ((Name != "strcpy") &&
+      (Name != "__strcpy_chk"))
+    return;
+
+  const FunctionProtoType *FPT
+    = dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens());
+  if (!FPT)
+    return;
+
+  // Verify the function takes two arguments
+  int numArgs = FPT->getNumArgs();
+  if (numArgs != 2 && numArgs != 3)
+    return;
+
+  // Verify the type for both arguments
+  for (int i = 0; i < 2; i++) {
+    // Verify that the arguments are pointers
+    const PointerType *PT = dyn_cast<PointerType>(FPT->getArgType(i));
+    if (!PT)
+      return;
+
+    // Verify that the argument is a 'char*'.
+    if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().CharTy)
+      return;
+  }
+
+  // Issue a warning
+  SourceRange R = CE->getCallee()->getSourceRange();
+  BR.EmitBasicReport("Potential insecure memory buffer bounds restriction in "
+		     "call 'strcpy'",
+		     "Security",
+		     "Call to function 'strcpy' is insecure as it does not "
+		     "provide bounding of the memory buffer. Replace "
+		     "unbounded copy functions with analogous functions that "
+		     "support length arguments such as 'strncpy'. CWE-119.",
+                     CE->getLocStart(), &R, 1);
+}
+
+//===----------------------------------------------------------------------===//
 // Check: Linear congruent random number generators should not be used
 // Originally: <rdar://problem/63371000>
 // CWE-338: Use of cryptographically weak prng
diff --git a/test/Analysis/security-syntax-checks.m b/test/Analysis/security-syntax-checks.m
index e08b08bff7..e342c21412 100644
--- a/test/Analysis/security-syntax-checks.m
+++ b/test/Analysis/security-syntax-checks.m
@@ -1,4 +1,16 @@
 // RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=security.experimental.SecuritySyntactic %s -verify
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -DUSE_BUILTINS -analyzer-checker=security.experimental.SecuritySyntactic %s -verify
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -DVARIANT -analyzer-checker=security.experimental.SecuritySyntactic %s -verify
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -DUSE_BUILTINS -DVARIANT -analyzer-checker=security.experimental.SecuritySyntactic %s -verify
+
+#ifdef USE_BUILTINS
+# define BUILTIN(f) __builtin_ ## f
+#else /* USE_BUILTINS */
+# define BUILTIN(f) f
+#endif /* USE_BUILTINS */
+
+typedef typeof(sizeof(int)) size_t;
+
 
 // <rdar://problem/6336718> rule request: floating point used as loop 
 //  condition (FLP30-C, FLP-30-CPP)
@@ -103,3 +115,28 @@ char *mktemp(char *buf);
 void test_mktemp() {
   char *x = mktemp("/tmp/zxcv"); // expected-warning{{Call to function 'mktemp' is insecure as it always creates or uses insecure temporary file}}
 }
+
+
+//===----------------------------------------------------------------------===
+// strcpy()
+//===----------------------------------------------------------------------===
+#ifdef VARIANT
+
+#define __strcpy_chk BUILTIN(__strcpy_chk)
+char *__strcpy_chk(char *restrict s1, const char *restrict s2, size_t destlen);
+
+#define strcpy(a,b) __strcpy_chk(a,b,(size_t)-1)
+
+#else /* VARIANT */
+
+#define strcpy BUILTIN(strcpy)
+char *strcpy(char *restrict s1, const char *restrict s2);
+
+#endif /* VARIANT */
+
+void test_strcpy() {
+  char x[4];
+  char *y;
+
+  strcpy(x, y); //expected-warning{{Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strncpy'. CWE-119.}}
+}