From: Anna Zaks Date: Wed, 24 Aug 2011 00:06:27 +0000 (+0000) Subject: [analyzer] MacOSKeychainAPIChecker: Add reasoning about functions which MIGHT dealloc... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6cf0ed062fb7ff3def3b627bab8ca275a549579e;p=clang [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. 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 7ffbc7fc4f..28f3336fbf 100644 --- 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(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(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(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(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(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 883268b941..69b36e811a 100644 --- 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); + } +}