An app I'm working on works with simple password items in the Login Keychain. I noticed there is a SecKeychainItemRef
that is never freed. The official documentation on SecKeychainFindGenericPassword()
reads:
On return, a pointer to the item object of the generic password. You are responsible for releasing your reference to this object.
After some quick changes, the static code analyzer claims:
- Assuming 'result' is equal to noErr
- Assuming 'item' is non-null
- Trying to free data which has not been allocated
I'm at a loss at how I'm assuming result is equal to noErr as there is an else clause. Not quite sure where I am assuming item is non-null nor how I am freeing data that is not allocated since it's checked (if (item)
).
Screenshot of the warnings and code listing
Code Listings
This is a part of the Hermes project on GitHub.
Old
BOOL KeychainSetItem(NSString* username, NSString* password) {
SecKeychainItemRef item;
OSStatus result = SecKeychainFindGenericPassword(
NULL,
strlen(KEYCHAIN_SERVICE_NAME),
KEYCHAIN_SERVICE_NAME,
[username length],
[username UTF8String],
NULL,
NULL,
&item);
if (result == noErr) {
result = SecKeychainItemModifyContent(item, NULL, [password length],
[password UTF8String]);
return result == noErr;
} else {
result = SecKeychainAddGenericPassword(
NULL,
strlen(KEYCHAIN_SERVICE_NAME),
KEYCHAIN_SERVICE_NAME,
[username length],
[username UTF8String],
[password length],
[password UTF8String],
NULL);
return result == noErr;
}
}
New
BOOL KeychainSetItem(NSString* username, NSString* password) {
SecKeychainItemRef item = nil;
OSStatus result = SecKeychainFindGenericPassword(
NULL,
strlen(KEYCHAIN_SERVICE_NAME),
KEYCHAIN_SERVICE_NAME,
[username length],
[username UTF8String],
NULL,
NULL,
&item);
if (result == noErr) {
result = SecKeychainItemModifyContent(item, NULL, [password length],
[password UTF8String]);
} else {
result = SecKeychainAddGenericPassword(
NULL,
strlen(KEYCHAIN_SERVICE_NAME),
KEYCHAIN_SERVICE_NAME,
[username length],
[username UTF8String],
[password length],
[password UTF8String],
NULL);
}
if (item) {
SecKeychainItemFreeContent(NULL, item);
}
return result == noErr;
}