Skip to content

macOS: set_generic_password leaks the SecKeychainItemRef, permanently breaking that process's reads after any other process saves #152

Description

Summary

msal_extensions.osx.Keychain.set_generic_password calls
SecKeychainFindGenericPassword to obtain a SecKeychainItemRef, passes it to
SecKeychainItemModifyAttributesAndData, and never releases it. The leaked ref
pins a stale ItemImpl in that process's Security.framework item cache, keyed on
service+account.

A Keychain secure storage item is not modified in place: saving deletes the
record and re-inserts it under a new UID. So as soon as any other process
saves the same item, the leaking process's pinned entry dangles, and every
subsequent read of that item in that process fails for the life of the
process
:

securityd:integrity  error while checking integrity, denying access:
CSSM Exception: -2147413720 CSSMERR_DL_INVALID_RECORD_UID

surfacing to the caller as OSStatus -67701 (errSecInvalidRecord), or -25300
(errSecItemNotFound) while the freed slot has not yet been reused. Nothing
evicts the pinned item, and the leaked ref would hold it anyway — there is no
in-process recovery.

This matters for the shared MSAL cache (Microsoft.Developer.IdentityService /
MSALCache), which is written by every process that mints or refreshes a token,
so multi-process setups poison each other by design. Each PersistedTokenCache
save leaks several refs.

The -25300 variant is the more confusing one: KeychainPersistence.load maps
it to PersistenceNotFound, and PersistedTokenCache._reload_if_necessary
swallows that with a bare pass. A denied read therefore presents as an
empty cacheget_accounts() returns [] — so the application reports "not
signed in" and re-prompts for a sign-in that was never the problem.

Affected

msal-extensions 1.3.1, and main (same code). macOS only.

Reproduction

Two processes, one Keychain item.

# writer.py — a second process saving the same item
import msal_extensions
p = msal_extensions.KeychainPersistence("/tmp/sig", "repro-service", "repro-account")
p.save('{"hello":"world"}')
# reader.py — long-lived, saves once then reads in a loop
import time, msal_extensions
p = msal_extensions.KeychainPersistence("/tmp/sig", "repro-service", "repro-account")
p.save('{"hello":"world"}')          # leaks a SecKeychainItemRef
for i in range(60):
    try:
        p.load()
        print(i, "ok")
    except Exception as exc:
        print(i, type(exc).__name__, getattr(exc, "exit_status", None))
    time.sleep(1)

Start reader.py, then run writer.py once. The reader prints ok until the
foreign save, then fails on every subsequent iteration (-25300 first, then
-67701 once the freed slot is reused). A reader that never calls save() is
unaffected, which isolates the leak as the cause.

Fix

Release the ref. In set_generic_password:

        if not find_exit_status:
            try:
                modify_exit_status = _SECURITY_KEYCHAIN_ITEM_MODIFY_ATTRIBUTES_AND_DATA(
                    entry, None, len(value), value,
                )
                if modify_exit_status:
                    raise KeychainError(exit_status=modify_exit_status)
            finally:
                _CORE_RELEASE(entry)

_CORE_RELEASE is already bound in the module. The add branch passes NULL
for the ref, so it needs no change. Applying this to the reader above makes it
survive foreign saves indefinitely.

Happy to open a PR if useful.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions