Security - inadequate PRNG and trivially bruteforced mnemonic password

[erwanor]

Hello,

Issues:

• The mnemonic is generated with the help of an insecure PRNG (https://github.com/mobius-network/wallet/blob/master/src/utils/generate-mnemonic-variations.js#L19) [Edit: actually on variations that are then discarded]

• The encrypted mnemonic can be trivially bruteforced

Since the pin is a numeric string of length 7, the space of potential candidates is only 10^7. The wallet is using AES192 to encrypt the mnemonic using the pin as a shared secret. (https://github.com/mobius-network/wallet/blob/master/src/utils/encrypt.js#L4)

  const cipher = crypto.createCipher('aes192', password);

Since the attacker knows that the first k bytes of the mnemonic are going to match at least one word in the list of variations (https://github.com/mobius-network/wallet/blob/master/src/utils/generate-mnemonic-variations.js#L11) they only need to decrypt the first block (16 bytes) of the cipher and make an additional comparison of the first k < 16 (an extremely low constant factor) bytes.

I think your reasoning was that the overhead incurred by generating a public key from the candidate seed would be enough to discourage bruteforcing. That is not the case and the fact that you use a very poor source of entropy compounds this. The security of the pin should rely on appropriate cryptographic primitives like PBKFs anyway.

It takes less than a minute for a (really) naive Golang proof-of-concept to break it on my machine:

Recommendations:

• Retire the wallet from the Apple/Android app stores until these issues are resolved

• Always use cryptographically secure PRNGs [Edit: as far as the underlying libraries are sound, this is the case]

• Closely follow BIP39 (https://github.com/bitcoin/bips/blob/master/bip-0039.mediawiki) do not make your own crypto. [Edit: this is the case]

• Increase the character set and length of the pin: alphanumeric string of 9 or 10 characters

• Use a combination of password key derivation functions (e.g PBKDF2 with a large enough number of rounds) and AES256-GCM

• Leverage secure enclaves/HSMs when the device allows it [Edit: this is at least partially the case]

• Last but not least: Hire professional cryptographers to audit your code and recommend solutions (e.g NCC Cryptography services, LeastAuthority, etc.). These are really serious issues. As part of the Stellar ecosystem, I am sure Mobius team can ask for feedback/recommendations from Interstellar/SDF.

Security vulnerabilities happens all the time. It is unfortunate but they don't define you. It is all about how you respond to them! (i.e don't sue me lol)

[nebolsin]

@aaronwinter Thanks for your report. We're still analyzing all the details, but I want to quickly point out that the insecure PRNG usage you're referring to has nothing to do with real mnemonic generation (which happens here https://github.com/mobius-network/wallet/blob/master/src/state/sagas/auth/signupStartSaga.js#L8). What generate-mnemonic-variations.js is used for is our implementation of "confirm that you actually saved the mnemonic" feature which offers a user to select a correct mnemonic from a few similar-looking phrases. The mnemonics generated there are immediately discarded after user completes the signup flow.

[erwanor]

No problem and thank you for the quick response. Glad to read that you fully rely on a BIP39-compliant library.

[dgobaud]

@aaronwinter thanks for the report. I think @nebolsin's comment addresses issue 1. On issue 2, yes a more complex password would be more secure but form a usability perspective we've decided requiring such a password is too big of a detriment to usability.

[erwanor]

You should increase the character set or password length - ideally both - and use primitives (i.e key-derivation functions) that make bruteforcing impractical. Otherwise you might as well not use AES at all since it takes less than a minute to crack.

[nebolsin]

Given that we have 6-digit pin as a product requirement (and I completely agree that it's suboptimal from security perspective), I don't think that using kdf's will make significant difference since the key space order is very low. What we can (and probably have to) do is to encrypt the BIP39 seed hex (basically, random 128-bit string) instead of mnemonic phrase as a text. This will slow the bruteforce significantly (for every attempt the attacker would have to decrypt the full ciphertext, then derive a wallet key via BIP44 process, and then check if such account exists on the Stellar network), but it still will not be very secure for 10^7 possible combinations, since a list of all existing accounts on the network can be easily cached in memory and decryption + BIP44 derivation + check will probably have sub-second execution time.

Having said that I would not consider this critical enough to retire the wallet from the Apple/Android app stores, because we only store the cyphertext using keychain service provided by the underlying platform, which is suitable to store sensitive data. So we mainly rely on the security of the mobile OS, and our PIN-based encryption is kind of superfluous.

Regarding Secure Enclave/HSMs, that would be our choice for sure, but unfortunately last time I checked there was no support for Ed25519 curve cryptography and HD key derivation on iOS/Android hardware, which makes this option unavailable for Stellar key management.

[erwanor]

This will slow the bruteforce significantly (for every attempt the attacker would have to decrypt the full ciphertext, then derive a wallet key via BIP44 process,

Yes, I agree this is a good step! There are two things:

As you pointed out, holding on such a small keyspace (10^6) makes any bruteforce mitigation impossible. It's just too small.

So regardless on whether you decide to rely on BIP44 CWs derivation alone or use it in addition to a large number of PBKDF2 (or bscrypt, argon2, etc. not sure which one would be optimal for your case) rounds for the AES shared-key, this effort will be a minor inconvenience to even an unsophisticated attacker.

Where I agree with you is that you do not want to encrypt the seed and its checksum. In the BIP39 specification, this value is called MS. Where MS = ENT_128 (concat) SHA-256(ENT_128)[0:3] (the total is 132 bits) and is the bitstring from which the mnemonic indexes are derived. The problem with this course of action is that an attacker can attempt every combinations and single out the AES keys which yield a decrypted candidate that has its last 4 bits matching the last 4 bits of SHA-256(ENT_128) where ENT_128 is your random 128 bits seed.

It's also unclear to me why you are limiting yourself to AES192 vs AES256-GCM. Not that it matters compared to the above issues though... AFAIK using 256 bits keys is the standard good practice in 2018.

I understand where you are coming from. You want to reduce friction as much as possible. At face value I think that’s fair. However, digging a little deeper alarms me on behalf of your users. Take that as my unsolicited non-expert opinion on the matter but I believe you should re-think it. At the very least, it should be made crystal clear to your users that you consider a weaker threat model than the advertisement makes it sound like.

The wallet is branded as "secure" but all user funds are at the mercy of a key extraction attack the moment it is found (or made public...) or of the device being rooted by a third-party. Is it such a good idea to rely entirely on the OS key store/keychain? Would you not rather decouple yourself as much as possible? There is only so much you can do in case of a downstream failure but one low hanging fruit here is simply buying your users time so that they can put their funds out of harm's way. Otherwise, losing your phone to the wrong person is tantamount to an immediate loss of funds. Is it worth it?

Regarding Secure Enclave/HSMs, that would be our choice for sure, but unfortunately last time I checked there was no support for Ed25519 curve cryptography and HD key derivation on iOS/Android hardware, which makes this option unavailable for Stellar key management.

Oh good point, apparently only secp256r1 is commonly supported. Too bad, it could have been nice.

As I mentioned earlier, I think you would be much better off hiring professional auditors with experience in mobile/embedded security. They will be able to provide you with guidance and relevant domain knowledge. Either they will tell you that my intuition here is wrong and the level of security provided is sufficient, or if it correct and there are in fact consequential attack vectors through which an attacker can pwn the end-user and steal their money.

It's midterm season for me - signing off this discussion! Good luck