[cfe-commits] r138417 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp test/Analysis/keychainAPI.m
Anna Zaks
ganna at apple.com
Tue Aug 23 17:06:27 PDT 2011
Author: zaks
Date: Tue Aug 23 19:06:27 2011
New Revision: 138417
URL: http://llvm.org/viewvc/llvm-project?rev=138417&view=rev
Log:
[analyzer] MacOSKeychainAPIChecker: Add reasoning about functions which MIGHT deallocate the memory region allocated with SecKeychain APIs. Specifically, when the buffer is passed to CFStringCreateWithBytesNoCopy along with a custom deallocator, which might potentially correctly release the memory.
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
cfe/trunk/test/Analysis/keychainAPI.m
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp?rev=138417&r1=138416&r2=138417&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp Tue Aug 23 19:06:27 2011
@@ -62,19 +62,25 @@
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 @@
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 @@
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 @@
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 @@
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;
}
Modified: cfe/trunk/test/Analysis/keychainAPI.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/keychainAPI.m?rev=138417&r1=138416&r2=138417&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/keychainAPI.m (original)
+++ cfe/trunk/test/Analysis/keychainAPI.m Tue Aug 23 19:06:27 2011
@@ -237,3 +237,70 @@
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);
+ }
+}
More information about the cfe-commits
mailing list