[analyzer] Add security checks for bcmp(), bcopy(), bzero().

These functions are obsolete. The analyzer would advice to replace them with
memcmp(), memcpy() or memmove(), or memset().

Patch by Tom Rix!

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

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

diff --git a/include/clang/StaticAnalyzer/Checkers/Checkers.td b/include/clang/StaticAnalyzer/Checkers/Checkers.td
index fdeeca0e3747f2cd1c0a073ae9f392db87d88ce2..eb91155d9de8edf854545f7c6f1399e18f2bf1d2 100644 (file)
--- a/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -373,6 +373,15 @@ def PaddingChecker : Checker<"Padding">,
 //===----------------------------------------------------------------------===//
 
 let ParentPackage = InsecureAPI in {
+  def bcmp : Checker<"bcmp">,
+    HelpText<"Warn on uses of the 'bcmp' function">,
+    DescFile<"CheckSecuritySyntaxOnly.cpp">;
+  def bcopy : Checker<"bcopy">,
+    HelpText<"Warn on uses of the 'bcopy' function">,
+    DescFile<"CheckSecuritySyntaxOnly.cpp">;
+  def bzero : Checker<"bzero">,
+    HelpText<"Warn on uses of the 'bzero' function">,
+    DescFile<"CheckSecuritySyntaxOnly.cpp">;
   def gets : Checker<"gets">,
     HelpText<"Warn on uses of the 'gets' function">,
     DescFile<"CheckSecuritySyntaxOnly.cpp">;

diff --git a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
index 62831be26eb2e6f8fe00def292e18b2d3ba8d9a4..15a13d8ac9959a1bf1637b4c32d440856b724258 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -37,6 +37,9 @@ static bool isArc4RandomAvailable(const ASTContext &Ctx) {
 
 namespace {
 struct ChecksFilter {
+  DefaultBool check_bcmp;
+  DefaultBool check_bcopy;
+  DefaultBool check_bzero;
   DefaultBool check_gets;
   DefaultBool check_getpw;
   DefaultBool check_mktemp;
@@ -47,6 +50,9 @@ struct ChecksFilter {
   DefaultBool check_FloatLoopCounter;
   DefaultBool check_UncheckedReturn;
 
+  CheckName checkName_bcmp;
+  CheckName checkName_bcopy;
+  CheckName checkName_bzero;
   CheckName checkName_gets;
   CheckName checkName_getpw;
   CheckName checkName_mktemp;
@@ -89,6 +95,9 @@ public:
 
   // Checker-specific methods.
   void checkLoopConditionForFloat(const ForStmt *FS);
+  void checkCall_bcmp(const CallExpr *CE, const FunctionDecl *FD);
+  void checkCall_bcopy(const CallExpr *CE, const FunctionDecl *FD);
+  void checkCall_bzero(const CallExpr *CE, const FunctionDecl *FD);
   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);
@@ -129,6 +138,9 @@ void WalkAST::VisitCallExpr(CallExpr *CE) {
 
   // Set the evaluation function by switching on the callee name.
   FnCheck evalFunction = llvm::StringSwitch<FnCheck>(Name)
+    .Case("bcmp", &WalkAST::checkCall_bcmp)
+    .Case("bcopy", &WalkAST::checkCall_bcopy)
+    .Case("bzero", &WalkAST::checkCall_bzero)
     .Case("gets", &WalkAST::checkCall_gets)
     .Case("getpw", &WalkAST::checkCall_getpw)
     .Case("mktemp", &WalkAST::checkCall_mktemp)
@@ -295,6 +307,132 @@ void WalkAST::checkLoopConditionForFloat(const ForStmt *FS) {
                      FSLoc, ranges);
 }
 
+//===----------------------------------------------------------------------===//
+// Check: Any use of bcmp.
+// CWE-477: Use of Obsolete Functions
+// bcmp was deprecated in POSIX.1-2008
+//===----------------------------------------------------------------------===//
+
+void WalkAST::checkCall_bcmp(const CallExpr *CE, const FunctionDecl *FD) {
+  if (!filter.check_bcmp)
+    return;
+
+  const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>();
+  if (!FPT)
+    return;
+
+  // Verify that the function takes three arguments.
+  if (FPT->getNumParams() != 3)
+    return;
+
+  for (int i = 0; i < 2; i++) {
+    // Verify the first and second argument type is void*.
+    const PointerType *PT = FPT->getParamType(i)->getAs<PointerType>();
+    if (!PT)
+      return;
+
+    if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().VoidTy)
+      return;
+  }
+
+  // Verify the third argument type is integer.
+  if (!FPT->getParamType(2)->isIntegralOrUnscopedEnumerationType())
+    return;
+
+  // Issue a warning.
+  PathDiagnosticLocation CELoc =
+    PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
+  BR.EmitBasicReport(AC->getDecl(), filter.checkName_bcmp,
+                     "Use of deprecated function in call to 'bcmp()'",
+                     "Security",
+                     "The bcmp() function is obsoleted by memcmp().",
+                     CELoc, CE->getCallee()->getSourceRange());
+}
+
+//===----------------------------------------------------------------------===//
+// Check: Any use of bcopy.
+// CWE-477: Use of Obsolete Functions
+// bcopy was deprecated in POSIX.1-2008
+//===----------------------------------------------------------------------===//
+
+void WalkAST::checkCall_bcopy(const CallExpr *CE, const FunctionDecl *FD) {
+  if (!filter.check_bcopy)
+    return;
+
+  const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>();
+  if (!FPT)
+    return;
+
+  // Verify that the function takes three arguments.
+  if (FPT->getNumParams() != 3)
+    return;
+
+  for (int i = 0; i < 2; i++) {
+    // Verify the first and second argument type is void*.
+    const PointerType *PT = FPT->getParamType(i)->getAs<PointerType>();
+    if (!PT)
+      return;
+
+    if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().VoidTy)
+      return;
+  }
+
+  // Verify the third argument type is integer.
+  if (!FPT->getParamType(2)->isIntegralOrUnscopedEnumerationType())
+    return;
+
+  // Issue a warning.
+  PathDiagnosticLocation CELoc =
+    PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
+  BR.EmitBasicReport(AC->getDecl(), filter.checkName_bcopy,
+                     "Use of deprecated function in call to 'bcopy()'",
+                     "Security",
+                     "The bcopy() function is obsoleted by memcpy() "
+                    "or memmove().",
+                     CELoc, CE->getCallee()->getSourceRange());
+}
+
+//===----------------------------------------------------------------------===//
+// Check: Any use of bzero.
+// CWE-477: Use of Obsolete Functions
+// bzero was deprecated in POSIX.1-2008
+//===----------------------------------------------------------------------===//
+
+void WalkAST::checkCall_bzero(const CallExpr *CE, const FunctionDecl *FD) {
+  if (!filter.check_bzero)
+    return;
+
+  const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>();
+  if (!FPT)
+    return;
+
+  // Verify that the function takes two arguments.
+  if (FPT->getNumParams() != 2)
+    return;
+
+  // Verify the first argument type is void*.
+  const PointerType *PT = FPT->getParamType(0)->getAs<PointerType>();
+  if (!PT)
+    return;
+
+  if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().VoidTy)
+    return;
+
+  // Verify the second argument type is integer.
+  if (!FPT->getParamType(1)->isIntegralOrUnscopedEnumerationType())
+    return;
+
+  // Issue a warning.
+  PathDiagnosticLocation CELoc =
+    PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
+  BR.EmitBasicReport(AC->getDecl(), filter.checkName_bzero,
+                     "Use of deprecated function in call to 'bzero()'",
+                     "Security",
+                     "The bzero() function is obsoleted by memset().",
+                     CELoc, CE->getCallee()->getSourceRange());
+}
+
+
 //===----------------------------------------------------------------------===//
 // Check: Any use of 'gets' is insecure.
 // Originally: <rdar://problem/6335715>
@@ -775,6 +913,9 @@ public:
     checker->filter.checkName_##name = mgr.getCurrentCheckName();              \
   }
 
+REGISTER_CHECKER(bcmp)
+REGISTER_CHECKER(bcopy)
+REGISTER_CHECKER(bzero)
 REGISTER_CHECKER(gets)
 REGISTER_CHECKER(getpw)
 REGISTER_CHECKER(mkstemp)

diff --git a/test/Analysis/security-syntax-checks.m b/test/Analysis/security-syntax-checks.m
index 8560ba29e70222614eb1893471efed376ce1e2d9..2c569727addd66683154713167ae3b4439094902 100644 (file)
--- a/test/Analysis/security-syntax-checks.m
+++ b/test/Analysis/security-syntax-checks.m
@@ -37,6 +37,27 @@ void test_float_condition() {
   for (FooType x = 100000001.0f; x <= 100000010.0f; x++ ) {} // expected-warning{{Variable 'x' with floating point type 'FooType'}}
 }
 
+// Obsolete function bcmp
+int bcmp(void *, void *, size_t);
+
+int test_bcmp(void *a, void *b, size_t n) {
+  return bcmp(a, b, n); // expected-warning{{The bcmp() function is obsoleted by memcmp()}}
+}
+
+// Obsolete function bcopy
+void bcopy(void *, void *, size_t);
+
+void test_bcopy(void *a, void *b, size_t n) {
+  bcopy(a, b, n); // expected-warning{{The bcopy() function is obsoleted by memcpy() or memmove(}}
+}
+
+// Obsolete function bzero
+void bzero(void *, size_t);
+
+void test_bzero(void *a, size_t n) {
+  bzero(a, n); // expected-warning{{The bzero() function is obsoleted by memset()}}
+}
+
 // <rdar://problem/6335715> rule request: gets() buffer overflow
 // Part of recommendation: 300-BSI (buildsecurityin.us-cert.gov)
 char* gets(char *buf);

diff --git a/www/analyzer/available_checks.html b/www/analyzer/available_checks.html
index b9c7846aa23d93b8c38d87b8f12d92abf35224ed..f764e0a19789e496988c8aa4b2afee05c41e6936 100644 (file)
--- a/www/analyzer/available_checks.html
+++ b/www/analyzer/available_checks.html
@@ -1172,6 +1172,40 @@ void test() {
 </pre></div></div></td></tr>
 
 
+<tr><td><div class="namedescr expandable"><span class="name">
+security.insecureAPI.bcmp</span><span class="lang">
+(C)</span><div class="descr">
+Warn on uses of the <code>bcmp</code> function.</div></div></td>
+<td><div class="exampleContainer expandable">
+<div class="example"><pre>
+void test() {
+  bcmp(ptr0, ptr1, n); // warn
+}
+</pre></div></div></td></tr>
+
+<tr><td><div class="namedescr expandable"><span class="name">
+security.insecureAPI.bcopy</span><span class="lang">
+(C)</span><div class="descr">
+Warn on uses of the <code>bcopy</code> function.</div></div></td>
+<td><div class="exampleContainer expandable">
+<div class="example"><pre>
+void test() {
+  bcopy(src, dst, n); // warn
+}
+</pre></div></div></td></tr>
+
+<tr><td><div class="namedescr expandable"><span class="name">
+security.insecureAPI.bzero</span><span class="lang">
+(C)</span><div class="descr">
+Warn on uses of the <code>bzero</code> function.</div></div></td>
+<td><div class="exampleContainer expandable">
+<div class="example"><pre>
+void test() {
+  bzero(ptr, n); // warn
+}
+</pre></div></div></td></tr>
+
+
 <tr><td><div class="namedescr expandable"><span class="name">
 security.insecureAPI.getpw</span><span class="lang">
 (C)</span><div class="descr">