Add security syntax checker for strcpy() which causes the Static Analyzer to generate...

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

diff --git a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
index 8b1756937e7e31ef215fe2a386dce0025942a983..61d3e4c652bbeb7827e8f0d8484a346b257b8fe5 100644 (file)
--- 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);
@@ -348,6 +350,58 @@ void WalkAST::CheckCall_mktemp(const CallExpr *CE, const FunctionDecl *FD) {
                      CE->getLocStart(), &R, 1);
 }
 
+//===----------------------------------------------------------------------===//
+// 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>

diff --git a/test/Analysis/security-syntax-checks.m b/test/Analysis/security-syntax-checks.m
index e08b08bff745b7ac305f3fd9c784676c4050e0a0..e342c214124ebe6d11a01e01b1d5f64963a6bcfe 100644 (file)
--- 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.}}
+}