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

Critical fix for a possibility of making own "password" or "private key" public by exposing it as a memo #1464

Closed
wants to merge 2 commits into from

Conversation

noisy
Copy link
Contributor

@noisy noisy commented Jun 7, 2017

We want to prevent such things:

selection_999 152

FIX:

This fix checks memo against public keys of a user, and prevents a mistake:

selection_999 148

selection_999 150

noisy added 2 commits June 7, 2017 16:10
…ers are using own private keys while making a transfers from an exchange), and they are still doing this, 'because this always worked'
@sneak
Copy link
Contributor

sneak commented Jun 7, 2017

Thanks for sending this in! We immediately undertook the same work as soon as we got the initial report of this happening, but it looks like you're faster. :)

I need to check and make sure that the .isWif function will detect a WIF format even if whitespace padded (e.g. 5.... or 5....) and the password check is too narrow - it should prevent anything that looks like a password, even if it's not the password for that specific account.

Do you want to make these changes, or should we? Again, thanks for the quick patch on this!

@noisy
Copy link
Contributor Author

noisy commented Jun 7, 2017

Do you want to make these changes, or should we? Again, thanks for the quick patch on this!

I think I can try to do that.

password check is too narrow - it should prevent anything that looks like a password, even if it's not the password for that specific account.

If this should be a matter of checking a pattern of /^5[HJK].{45,}/i (I will take a look into source whether this can be more detail check) - than this will be no problem. But technically everything can be a password if someone set a password before last hack, or set own password using console - so I believe, we talking only about checking a pattern?

@sneak
Copy link
Contributor

sneak commented Jun 7, 2017

Turns out @mcifani already had this mostly done on his local version and it's tested and merged ( #1467 ) and staged now - should be rolled out to prod within a couple hours. :)

Great minds think alike - thank you for jumping in on this!

@sneak sneak closed this Jun 7, 2017
@sneak
Copy link
Contributor

sneak commented Jun 7, 2017

PS: https://steemitstage.com (not HA, not to be relied upon - but you can see the fix live)

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.

2 participants