[PATCH] D54592: [CStringChecker] evaluate explicit_bzero

[David CARLIER via Phabricator via cfe-commits]

devnexen created this revision.
devnexen added reviewers: george.karpenkov, dergachev.a.
devnexen created this object with visibility "All Users".
Herald added a subscriber: cfe-commits.

- explicit_bzero has limited scope/usage only for security/crypto purposes but 
is non-optimisable version of memset/0 and bzero.
- explicit_memset has similar signature and semantics as memset but is also a 
non-optimisable version.


Repository:
  rC Clang

https://reviews.llvm.org/D54592

Files:
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  test/Analysis/string.c


Index: test/Analysis/string.c
===================================================================
--- test/Analysis/string.c
+++ test/Analysis/string.c
@@ -1184,11 +1184,13 @@
 }
 
 //===----------------------------------------------------------------------===
-// memset()
+// memset() / explicit_bzero()
 //===----------------------------------------------------------------------===
 
 void *memset(void *dest, int ch, size_t count);
 
+void explicit_bzero(void *dest, size_t count);
+
 void *malloc(size_t size);
 void free(void *);
 
@@ -1383,6 +1385,32 @@
   clang_analyzer_eval(array[4] == 0); // expected-warning{{TRUE}}
 }
 
+void explicit_bzero1_null() {
+  char *a = NULL;
+
+  explicit_bzero(a, 10); // expected-warning{{Null pointer argument in call to memory clearance function}}
+}
+
+void explicit_bzero2_clear_mypassword() {
+  char passwd[7] = "passwd";
+
+  explicit_bzero(passwd, sizeof(passwd)); // no-warning
+
+  clang_analyzer_eval(strlen(passwd) == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(passwd[0] == '\0'); // expected-warning{{UNKNOWN}}
+}
+
+#ifdef SUPPRESS_OUT_OF_BOUND
+void explicit_bzero3_out_ofbound() {
+  char *privkey = (char *)malloc(6);
+  const char newprivkey[10] = "mysafekey";
+
+  strcpy(privkey, "random");
+  explicit_bzero(privkey, sizeof(newprivkey));
+  clang_analyzer_eval(privkey[0] == '\0'); // expected-warning{{UNKNOWN}}
+  free(privkey);
+}
+#endif
 //===----------------------------------------------------------------------===
 // FIXMEs
 //===----------------------------------------------------------------------===
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -124,6 +124,7 @@
   void evalStdCopyBackward(CheckerContext &C, const CallExpr *CE) const;
   void evalStdCopyCommon(CheckerContext &C, const CallExpr *CE) const;
   void evalMemset(CheckerContext &C, const CallExpr *CE) const;
+  void evalExplicitBzero(CheckerContext &C, const CallExpr *CE) const;
 
   // Utility methods
   std::pair<ProgramStateRef , ProgramStateRef >
@@ -2191,6 +2192,44 @@
   C.addTransition(State);
 }
 
+void CStringChecker::evalExplicitBzero(CheckerContext &C, const CallExpr *CE) const {
+  if (CE->getNumArgs() != 2)
+    return;
+
+  CurrentFunctionDescription = "memory clearance function";
+
+  const Expr *Mem = CE->getArg(0);
+  const Expr *Size = CE->getArg(1);
+  ProgramStateRef State = C.getState();
+  
+  // See if the size argument is zero.
+  const LocationContext *LCtx = C.getLocationContext();
+  SVal SizeVal = State->getSVal(Size, LCtx);
+  QualType SizeTy = Size->getType();
+
+  ProgramStateRef StateZeroSize, StateNonZeroSize;
+  std::tie(StateZeroSize, StateNonZeroSize) =
+    assumeZero(C, State, SizeVal, SizeTy);
+
+  // Get the value of the memory area.
+  SVal MemVal = State->getSVal(Mem, LCtx);
+  
+  // If the size is zero, there won't be any actual memory access,
+  // In this case we just return.
+  if (StateZeroSize && !StateNonZeroSize)
+    return;
+
+  // Ensure the memory area is not null.
+  // If it is NULL there will be a NULL pointer dereference.
+  State = checkNonNull(C, StateNonZeroSize, Mem, MemVal);
+  if (!State)
+    return;
+
+  State = CheckBufferAccess(C, State, Size, Mem);
+  if (!State)
+    return;
+}
+
 static bool isCPPStdLibraryFunction(const FunctionDecl *FD, StringRef Name) {
   IdentifierInfo *II = FD->getIdentifier();
   if (!II)
@@ -2224,7 +2263,8 @@
     evalFunction =  &CStringChecker::evalMemcmp;
   else if (C.isCLibraryFunction(FDecl, "memmove"))
     evalFunction =  &CStringChecker::evalMemmove;
-  else if (C.isCLibraryFunction(FDecl, "memset"))
+  else if (C.isCLibraryFunction(FDecl, "memset") || 
+    C.isCLibraryFunction(FDecl, "explicit_memset"))
     evalFunction =  &CStringChecker::evalMemset;
   else if (C.isCLibraryFunction(FDecl, "strcpy"))
     evalFunction =  &CStringChecker::evalStrcpy;
@@ -2262,6 +2302,8 @@
     evalFunction =  &CStringChecker::evalStdCopy;
   else if (isCPPStdLibraryFunction(FDecl, "copy_backward"))
     evalFunction =  &CStringChecker::evalStdCopyBackward;
+  else if (C.isCLibraryFunction(FDecl, "explicit_bzero"))
+    evalFunction =  &CStringChecker::evalExplicitBzero;
 
   // If the callee isn't a string function, let another checker handle it.
   if (!evalFunction)

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits