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:
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)
).
This is a part of the Hermes project on GitHub.
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;
}
}
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;
}
SecKeychainItemRef
variables are CoreFoundation reference counted. From the static code analyzer's point of view, calling SecKeychainItemFreeContent()
on a SecKeychainItemRef
amounts to freeing unallocated data since it was not allocated by the SecKeychain functions.
All the errors went away by calling CFRelease()
on the SecKeychainItemRef
variable (instead of SecKeychainItemFreeContent()
).