Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Memory Leak in iOS KeychainItemWrapper

I'm using the KeyChainItemWrapper from Apple's sample code to store user password for authentication, but when I call it to set the password:

   [keychain setObject:passwordField.text forKey:(id)kSecValueData];

It dribbles memory leaks all over my shirt. The problem apparently traces back to line 274 in KeyChainItemWrapper.m, which is this:

if (SecItemCopyMatching((CFDictionaryRef)genericPasswordQuery, (CFTypeRef *)&attributes) == noErr)
{

How would I fix this, and should I be more careful when working with Apple sample code in the future?

Note: I could post more code, but I've narrowed the problem down to this line using Instruments and the full sample code is readily available to any developer.

like image 912
Serendipity Avatar asked Dec 29 '11 07:12

Serendipity


People also ask

How detect memory leak iOS app?

Xcode's Memory Graph Debugger If you haven't used this yet, it's easy to access while developing. Tapping on the icon will pause your application and generate a graph of the objects with their references to other objects. If there's leaked memory detected, you will see purple icons on the left pane of Xcode.

How memory leak happens in iOS?

Memory that was allocated at some point, but was never released and is no longer referenced by your app. Since there are no references to it, there's now no way to release it and the memory can't be used again. So a memory leak occurs when a content remains in memory even after its life cycle has ended.

What is a memory leak Swift?

A memory leak occurs in an iOS app when memory that is no longer in use is not properly cleared and continues taking up space. This can hurt app performance, and eventually, when the app runs out of available memory, will cause a crash.


2 Answers

Looking at the code for KeyChainItemWrapper, I'd agree that this line is a memory leak. They missed the [attributes release] at the end of writeToKeychain. See all the other calls to SecItemCopyMatching() in this file for examples of how they correctly release the returned-by-reference object.

I would use the "It's good, but..." link at the bottom of this page to note the error.

like image 173
Rob Napier Avatar answered Sep 19 '22 14:09

Rob Napier


Static analysis is reporting a potential leak of on object in method resetKeychainItem, line 191, of KeyChainItemWrapper.m. Surprisingly, it does not report a potential leak in the area addressed above, although I did add release of the object as suggested, and for correctness.

Here is the code with the leak being reported:

- (void)resetKeychainItem
{
    ...
    // Default attributes for keychain item.
    [keychainItemData setObject:@"" forKey:(id)kSecAttrAccount]; // <-- Potential leak of an object
    [keychainItemData setObject:@"" forKey:(id)kSecAttrLabel];
    [keychainItemData setObject:@"" forKey:(id)kSecAttrDescription];

    // Default data for keychain item.
    [keychainItemData setObject:@"" forKey:(id)kSecValueData];
}

This issue is being reported on the empty string @"". I tried a variety of code implementations to try and "fix" this issue, but nothing appears to work.

Is this is a false positive?

Update: Right after posting I realized that I can double-click on the warning to trace the error.

This warning is attributed to the line above it for allocating the dictionary:

if (!keychainItemData)
{
    self.keychainItemData = [[NSMutableDictionary alloc] init];
}

I changed the code to the following:

if (!keychainItemData)
{
    self.keychainItemData = [[[NSMutableDictionary alloc] init] autorelease];
}

The analyzer warning is no longer present.

like image 42
Christopher Avatar answered Sep 18 '22 14:09

Christopher