Frage

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:

  1. Assuming 'result' is equal to noErr
  2. Assuming 'item' is non-null
  3. 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

Screenshot of the warning 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;
}
War es hilfreich?

Lösung

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()).

Lizenziert unter: CC-BY-SA mit Zuschreibung
Nicht verbunden mit StackOverflow
scroll top