sebsauvage / ZeroBin

A minimalist, opensource online pastebin where the server has zero knowledge of pasted data. Data is encrypted/decrypted in the browser using 256 bits AES.
http://sebsauvage.net/wiki/doku.php?id=php:zerobin
952 stars 213 forks source link

VizHash Exploit #68

Closed SoniEx2 closed 10 years ago

SoniEx2 commented 10 years ago

As the FAQ says, the VizHash salt is server-side... This means all comments use the same salt... Someone could grab their IP + their VizHash and explore/bruteforce 504 bits instead of 536, and then all the IPs can be explored/bruteforced as 32 bits...

(Please note that I don't know how VizHashes really work so sorry if I'm wrong...)

CodesInChaos commented 10 years ago

You cannot brute-force 504 bits unless you have additional information about them. Even 128 bits far exceeds what all of humanity can currently achieve.

SoniEx2 commented 10 years ago

I can bruteforce the 504 bits as 32 bits, actually... (because mt_rand())

SoniEx2 commented 10 years ago

This could be bad... https://github.com/sebsauvage/ZeroBin/blob/master/index.php#L288

Zolmeister commented 10 years ago

I don't see how it could be 32 bits. Check this line: https://github.com/sebsauvage/ZeroBin/blob/master/lib/serversalt.php#L7 It's 32 bits * 16 because of the loop.

CodesInChaos commented 10 years ago

@Zolmeister

It doesn't matter how many bits you draw from a PRNG, you can't get a higher security level than the seed. For example when you seed with a 32 bit value, there will only be 4 billion different output sequences, even if those are longer than 32 bits.
I believe mt_rand uses a 32 bit seed, which is even easier to guess than a random 32 bit value, since it's based on the time. 32 bit values collide pretty often as well, so they're not suitable as salts.

You should use a CRPNG to generate the salt. I know three ways to do that in php:

  1. mcrypt_create_iv(size, MCRYPT_DEV_URANDOM) (don't use one of the other sources, they're either broken or extremely slow) despite the name, this works on windows
  2. openssl_random_pseudo_bytes is tricky to use since it can return low quality outputs, which you need to check for.
  3. Manually read from /dev/urandom (Linux only)

I recommend using mcrypt_create_iv(size, MCRYPT_DEV_URANDOM).

Zolmeister commented 10 years ago

@CodesInChaos Thanks, you inspired me to look into PRNG cracking. If you have a minute could you please explain how it would be feasible to attack the server externally to crack the prng? From what I understand, you only need a small sample from the random numbers, but it is only feasible if they are the first random numbers generated with the seed (which in this case I think they are, so it's not a problem). If you don't seed mt_rand(), does it pick a seed at random? (and each time, or just the first time) - if it's each time then running it 16 times would mean cracking 16 separate PRNG seeds right?

Here's what I was thinking (let me know if I've missed something):

  1. we assume that mt_rand() is seeded once to start, and that the 16 random times it is called are the first 16 random numbers generated
  2. get 1 deletion token (sha1) from the server, and we have the data_id (gotten somehow)
  3. we crack all 32 bit seeds, by getting the first 16 generated numbers, adding our data_id, and comparing to the sha1
  4. once we have a match, we have our seed

Is there a way to work around not having the data_id?

Wikinaut commented 10 years ago

see http://www.openwall.com/php_mt_seed/

SoniEx2 commented 10 years ago

@Zolmeister data_id = paste ID

st1gma commented 10 years ago

Soni, you are correct. mt_rand should be replaced as it is not a secure random generator. As you observed you could crack a bigger key using smaller 32bit instances or 64bit instances (depending on 32 or 64 bit php compilation).

Also the generation of the HMAC is ok, I'd probably move SHA1 to be SHA256 but other than that it should be ok. SHA1 has theoretical collisions but the numbers are astronomical making the true use of them very impractical.

sebsauvage commented 10 years ago

@CodesInChaos: Thanks. mcrypt_create_iv(size, MCRYPT_DEV_URANDOM) will be added as default, with a fallback to mt_rand() when not available.

sebsauvage commented 10 years ago

This issue has been fixed in commit https://github.com/sebsauvage/ZeroBin/commit/a24212afda90ca3e4b4ff5ce30d2012709b58a28 Thank you for helping me to improve ZeroBin security.