adyliu / jeos

EOS Blockchain Java SDK
https://github.com/adyliu/jeos
71 stars 38 forks source link

Private key generation security issue #7

Open cyberluke opened 5 years ago

cyberluke commented 5 years ago

Ni hao, you have bad practices in your KeyUtil.

1) UUID is not recommended for other stuff than userId. It was used back in the day for content management systems in Java. It is definitely not for generating random numbers (read specification)

2) UUID length is 16 bytes. Plus some bytes are static, not random. For example it also contains version byte. So you loose entropy here.

3) Next you put UUID string into SHA-256. But you will not get any extra security from converting 16 bytes to 32 bytes here. You also loose entropy level here.

UUID is timebased, low entropy, pseudorandom. High probability of conflict. High probability to be traced back by timestamp. Therefore if you use UUID and create cryptocurrency account, it is easily hackable and you loose all your money.


public static UUID randomUUID() {
        SecureRandom ng = Holder.numberGenerator;

        byte[] randomBytes = new byte[16];
        ng.nextBytes(randomBytes);
        randomBytes[6]  &= 0x0f;  /* clear version        */
        randomBytes[6]  |= 0x40;  /* set to version 4     */
        randomBytes[8]  &= 0x3f;  /* clear variant        */
        randomBytes[8]  |= 0x80;  /* set to IETF variant  */
        return new UUID(randomBytes);
    }`
cyberluke commented 5 years ago

Instead of this: byte[] b = new BigInteger(SHA.sha256(UUID.randomUUID())).toByteArray();

You need to generate real 32 random bytes or at least pseudo random, but it must be 32 bytes, not less.

samkirton commented 5 years ago

Please see https://github.com/memtrip/eos-jvm, the key generation is based off bitcoinJ.

cyberluke commented 5 years ago

I was working on BitcoinJ as well and it was not like that here. Something is wrong. The UUID is based on timestamp. It is really only for user id generation for content management system and Oracle documentation mentions it.

cyberluke commented 5 years ago

We couldn’t find any code matching 'uuid' in memtrip/eos-jvm ...guess they don't have it as well.