Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(pii): Add private keys as secret key name #1376

Merged
merged 3 commits into from
Aug 4, 2022

Conversation

untitaker
Copy link
Member

@untitaker untitaker commented Aug 3, 2022

https://twitter.com/MoonRankNFT/status/1554911833617641472/photo/1

Its been reported that someone was - likely accidentally - sending a private key to their Sentry instance. There's not a great use case to allow storing that kind of value, so we are adding it to our default blocklists.

The hashes and "mnemonic" seem not very useful for pattern matching, but
we can at least filter out the string if it contains the word
"privatekey" (case-insensitive)

Somebody was sending a private key to their Sentry instance via
breadcrumbs:

https://twitter.com/MoonRankNFT/status/1554911833617641472/photo/1

The hashes and "mnemonic" seem not very useful for pattern matching, but
we can at least filter out the string if it contains the word
"privatekey" (case-insensitive)
@untitaker untitaker requested a review from a team August 3, 2022 21:01
@lucas-zimerman
Copy link

@untitaker mnemonic is the seed that's used to generate the private key (that was also included on the payload).
Would you mind if I add that on another PR?

@jan-auer jan-auer changed the title fix(pii): Add one more secret key name fix(pii): Add private keys as secret key name Aug 4, 2022
@jan-auer jan-auer enabled auto-merge (squash) August 4, 2022 05:43
@jan-auer jan-auer merged commit 4a24fbe into master Aug 4, 2022
@jan-auer jan-auer deleted the fix/more-secret-key-names branch August 4, 2022 05:59
@untitaker
Copy link
Member Author

@lucas-zimerman how? I didn't see any pattern there we could detect. But yeah if you have an idea feel free to send a PR

@untitaker
Copy link
Member Author

also btw keep in mind. with this PR, the entire json string will be deleted anyway, because it contains private_key

@lucas-zimerman
Copy link

@lucas-zimerman how? I didn't see any pattern there we could detect. But yeah if you have an idea feel free to send a PR

image

Basically the mnemonic phrase, is made up of 12, 18, or 24 words, based on BIP39,. TL;DR It can be used to generate a Private key.

also btw keep in mind. with this PR, the entire json string will be deleted anyway, because it contains private_key

Indeed, The entire json string will be removed ,but the user could remove the private_key and keep the mnemonic on the payload, leading to the private key leak.

@untitaker
Copy link
Member Author

untitaker commented Aug 5, 2022

Basically the mnemonic phrase, is made up of 12, 18, or 24 words, based on BIP39,. TL;DR It can be used to generate a Private key.

I see, so there's a standard with predefined words. I think if you want to put up a PR for this feel free to. I suspect the regex will be large and imperformant.

We could denylist the word mnemonic as well 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants