Repository navigation
Zero the native plaintext TryGet loads before freeing it [patch] - #209
Open
matt-edmondson wants to merge 1 commit into
Open
matt-edmondson wants to merge 1 commit into
matt-edmondson wants to merge 1 commit into
Conversation
On Windows and macOS, TryLoad copied the decrypted credential out of the buffer CredRead / SecKeychainFindGenericPassword returned and scrubbed only that managed copy. CredFree and SecKeychainItemFreeContent then released the native block unscrubbed, leaving a plaintext copy in freed heap on every load. NativeSecretBuffer.ReadCredentialAndZero now reads, deserializes and scrubs the managed copy, and zeroes the native bytes on every path out while they are still allocated. Both stores read through it, and a wiring test pins them to it, since neither store body runs on the Linux CI leg. Fixes #173 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW5TgsM2XbL48rHXNad
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #173
What was wrong
On a cache miss,
TryGetcallsStore.TryLoad. On Windows and macOS the store copied the decrypted credential out of the native buffer (CredRead'sCredentialBlob,SecKeychainFindGenericPassword'spasswordData) and scrubbed only that managed copy.CredFree/SecKeychainItemFreeContentthen released the native block without zeroing it, so every load left a plaintext copy of the secret in freed heap.Change
NativeSecretBuffer.ReadCredentialAndZero(pointer, length)copies the bytes, deserializes them throughDeserializeAndScrub, and zeroes the native bytes in afinally. The memory is still allocated at that point, the unparseable path is covered, and the caller frees it afterwards.WindowsCredentialStore.TryLoadandMacOsCredentialStore.TryLoadread through it. Linux is unchanged, becausesecret_password_freealready wipes its copy.The helper is shared, like the other primitives, so its behaviour runs under test on every OS.
Tests
SecretScrubbingTests: the helper returns the credential and zeroes the native copy, zeroes it for unparseable bytes too, and returns null for a null pointer or zero length without touching memory.NativeStoreScrubbingWiringTests.TheWindowsAndMacOsStoresZeroTheNativePlaintextTheyLoadIL-pins both stores to the helper. It fails with the store changes reverted and passes with them. The existing load-wiring and second-hop assertions now also accept the new wrapper.Generated by Claude Code