[analyzer] MacOSKeychainAPIChecker: Add reasoning about functions which MIGHT dealloc...

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

diff --git a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
index 7ffbc7fc4f76fe3f71614108565e9067feb2667f..28f3336fbf27c543cffc24faedac5e322f5ce966 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
@@ -62,19 +62,25 @@ public:
   void checkEndPath(EndOfFunctionNodeBuilder &B, ExprEngine &Eng) const;
 
 private:
+  enum APIKind{
+    /// Denotes functions tracked by this checker.
+    ValidAPI = 0,
+    /// The functions commonly/mistakenly used in place of the given API.
+    ErrorAPI = 1,
+    /// The functions which may allocate the data. These are tracked to reduce
+    /// the false alarm rate.
+    PossibleAPI = 2
+  };
   /// Stores the information about the allocator and deallocator functions -
   /// these are the functions the checker is tracking.
   struct ADFunctionInfo {
     const char* Name;
     unsigned int Param;
     unsigned int DeallocatorIdx;
-    /// The flag specifies if the call is valid or is a result of a common user
-    /// error (Ex: free instead of SecKeychainItemFreeContent), which we also
-    /// track for better diagnostics.
-    bool isValid;
+    APIKind Kind;
   };
   static const unsigned InvalidIdx = 100000;
-  static const unsigned FunctionsToTrackSize = 7;
+  static const unsigned FunctionsToTrackSize = 8;
   static const ADFunctionInfo FunctionsToTrack[FunctionsToTrackSize];
   /// The value, which represents no error return value for allocator functions.
   static const unsigned NoErr = 0;
@@ -138,13 +144,14 @@ static bool isEnclosingFunctionParam(const Expr *E) {
 
 const MacOSKeychainAPIChecker::ADFunctionInfo
   MacOSKeychainAPIChecker::FunctionsToTrack[FunctionsToTrackSize] = {
-    {"SecKeychainItemCopyContent", 4, 3, true},                       // 0
-    {"SecKeychainFindGenericPassword", 6, 3, true},                   // 1
-    {"SecKeychainFindInternetPassword", 13, 3, true},                 // 2
-    {"SecKeychainItemFreeContent", 1, InvalidIdx, true},              // 3
-    {"SecKeychainItemCopyAttributesAndData", 5, 5, true},             // 4
-    {"SecKeychainItemFreeAttributesAndData", 1, InvalidIdx, true},    // 5
-    {"free", 0, InvalidIdx, false},                                   // 6
+    {"SecKeychainItemCopyContent", 4, 3, ValidAPI},                       // 0
+    {"SecKeychainFindGenericPassword", 6, 3, ValidAPI},                   // 1
+    {"SecKeychainFindInternetPassword", 13, 3, ValidAPI},                 // 2
+    {"SecKeychainItemFreeContent", 1, InvalidIdx, ValidAPI},              // 3
+    {"SecKeychainItemCopyAttributesAndData", 5, 5, ValidAPI},             // 4
+    {"SecKeychainItemFreeAttributesAndData", 1, InvalidIdx, ValidAPI},    // 5
+    {"free", 0, InvalidIdx, ErrorAPI},                                    // 6
+    {"CFStringCreateWithBytesNoCopy", 1, InvalidIdx, PossibleAPI},        // 7
 };
 
 unsigned MacOSKeychainAPIChecker::getTrackedFunctionIndex(StringRef Name,
@@ -298,9 +305,6 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE,
   if (idx == InvalidIdx)
     return;
 
-  // We also track invalid deallocators. Ex: free() for enhanced error messages.
-  bool isValidDeallocator = FunctionsToTrack[idx].isValid;
-
   // Check the argument to the deallocator.
   const Expr *ArgExpr = CE->getArg(FunctionsToTrack[idx].Param);
   SVal ArgSVal = State->getSVal(ArgExpr);
@@ -320,11 +324,15 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE,
   if (!ArgSM && !RegionArgIsBad)
     return;
 
+  // Is the argument to the call being tracked?
+  const AllocationState *AS = State->get<AllocatedData>(ArgSM);
+  if (!AS && FunctionsToTrack[idx].Kind != ValidAPI) {
+    return;
+  }
   // If trying to free data which has not been allocated yet, report as a bug.
   // TODO: We might want a more precise diagnostic for double free
   // (that would involve tracking all the freed symbols in the checker state).
-  const AllocationState *AS = State->get<AllocatedData>(ArgSM);
-  if ((!AS || RegionArgIsBad) && isValidDeallocator) {
+  if (!AS || RegionArgIsBad) {
     // It is possible that this is a false positive - the argument might
     // have entered as an enclosing function parameter.
     if (isEnclosingFunctionParam(ArgExpr))
@@ -341,13 +349,46 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE,
     return;
   }
 
+  // Process functions which might deallocate.
+  if (FunctionsToTrack[idx].Kind == PossibleAPI) {
+
+    if (funName == "CFStringCreateWithBytesNoCopy") {
+      const Expr *DeallocatorExpr = CE->getArg(5)->IgnoreParenCasts();
+      // NULL ~ default deallocator, so warn.
+      if (DeallocatorExpr->isNullPointerConstant(C.getASTContext(),
+          Expr::NPC_ValueDependentIsNotNull)) {
+        generateDeallocatorMismatchReport(*AS, ArgExpr, C, ArgSM);
+        return;
+      }
+      // One of the default allocators, so warn.
+      if (const DeclRefExpr *DE = dyn_cast<DeclRefExpr>(DeallocatorExpr)) {
+        StringRef DeallocatorName = DE->getFoundDecl()->getName();
+        if (DeallocatorName == "kCFAllocatorDefault" ||
+            DeallocatorName == "kCFAllocatorSystemDefault" ||
+            DeallocatorName == "kCFAllocatorMalloc") {
+          generateDeallocatorMismatchReport(*AS, ArgExpr, C, ArgSM);
+          return;
+        }
+        // If kCFAllocatorNull, which does not deallocate, we still have to
+        // find the deallocator. Otherwise, assume that the user had written a
+        // custom deallocator which does the right thing.
+        if (DE->getFoundDecl()->getName() != "kCFAllocatorNull") {
+          State = State->remove<AllocatedData>(ArgSM);
+          C.addTransition(State);
+          return;
+        }
+      }
+    }
+    return;
+  }
+
   // The call is deallocating a value we previously allocated, so remove it
   // from the next state.
   State = State->remove<AllocatedData>(ArgSM);
 
   // Check if the proper deallocator is used.
   unsigned int PDeallocIdx = FunctionsToTrack[AS->AllocatorIdx].DeallocatorIdx;
-  if (PDeallocIdx != idx || !isValidDeallocator) {
+  if (PDeallocIdx != idx || (FunctionsToTrack[idx].Kind == ErrorAPI)) {
     generateDeallocatorMismatchReport(*AS, ArgExpr, C, ArgSM);
     return;
   }

diff --git a/test/Analysis/keychainAPI.m b/test/Analysis/keychainAPI.m
index 883268b9413dad6bfa70aac1bd9fcd9011b6dab8..69b36e811a58b6296500e051f887643cecce7208 100644 (file)
--- a/test/Analysis/keychainAPI.m
+++ b/test/Analysis/keychainAPI.m
@@ -237,3 +237,70 @@ void deallocateWithFree() {
       free(outData); // expected-warning{{Deallocator doesn't match the allocator: 'SecKeychainItemFreeContent' should be used}}
 }
 
+// Typesdefs for CFStringCreateWithBytesNoCopy.
+typedef char uint8_t;
+typedef signed long CFIndex;
+typedef UInt32 CFStringEncoding;
+typedef unsigned Boolean;
+typedef const struct __CFString * CFStringRef;
+typedef const struct __CFAllocator * CFAllocatorRef;
+extern const CFAllocatorRef kCFAllocatorDefault;
+extern const CFAllocatorRef kCFAllocatorSystemDefault;
+extern const CFAllocatorRef kCFAllocatorMalloc;
+extern const CFAllocatorRef kCFAllocatorMallocZone;
+extern const CFAllocatorRef kCFAllocatorNull;
+extern const CFAllocatorRef kCFAllocatorUseContext;
+CFStringRef CFStringCreateWithBytesNoCopy(CFAllocatorRef alloc, const uint8_t *bytes, CFIndex numBytes, CFStringEncoding encoding, Boolean externalFormat, CFAllocatorRef contentsDeallocator);
+extern void CFRelease(CFStringRef cf);
+
+void DellocWithCFStringCreate1(CFAllocatorRef alloc) {
+  unsigned int *ptr = 0;
+  OSStatus st = 0;
+  UInt32 length;
+  void *bytes;
+  char * x;
+  st = SecKeychainItemCopyContent(2, ptr, ptr, &length, &bytes);
+  if (st == noErr) {
+    CFStringRef userStr = CFStringCreateWithBytesNoCopy(alloc, bytes, length, 5, 0, kCFAllocatorDefault); // expected-warning{{Deallocator doesn't match the allocator:}} 
+    CFRelease(userStr);
+  }
+}
+
+void DellocWithCFStringCreate2(CFAllocatorRef alloc) {
+  unsigned int *ptr = 0;
+  OSStatus st = 0;
+  UInt32 length;
+  void *bytes;
+  char * x;
+  st = SecKeychainItemCopyContent(2, ptr, ptr, &length, &bytes);
+  if (st == noErr) {
+    CFStringRef userStr = CFStringCreateWithBytesNoCopy(alloc, bytes, length, 5, 0, kCFAllocatorNull); // expected-warning{{Allocated data is not released}}
+    CFRelease(userStr);
+  }
+}
+
+void DellocWithCFStringCreate3(CFAllocatorRef alloc) {
+  unsigned int *ptr = 0;
+  OSStatus st = 0;
+  UInt32 length;
+  void *bytes;
+  char * x;
+  st = SecKeychainItemCopyContent(2, ptr, ptr, &length, &bytes);
+  if (st == noErr) {
+    CFStringRef userStr = CFStringCreateWithBytesNoCopy(alloc, bytes, length, 5, 0, kCFAllocatorUseContext);
+    CFRelease(userStr);
+  }
+}
+
+void DellocWithCFStringCreate4(CFAllocatorRef alloc) {
+  unsigned int *ptr = 0;
+  OSStatus st = 0;
+  UInt32 length;
+  void *bytes;
+  char * x;
+  st = SecKeychainItemCopyContent(2, ptr, ptr, &length, &bytes);
+  if (st == noErr) {
+    CFStringRef userStr = CFStringCreateWithBytesNoCopy(alloc, bytes, length, 5, 0, 0); // expected-warning{{Deallocator doesn't match the allocator:}} 
+    CFRelease(userStr);
+  }
+}