Remote Code Execution about n2o HOT 9 CLOSED

Please use {n2o,[{pickler,n2o_secure}]} in sys.config for enabling AES-CBC/RIPEMD pickling.
By default N2O uses transparent proxy model.

from n2o.

Thanks :) I missed this.

Looking at the crypto it seems to be broken.

https://github.com/5HT/n2o/blob/c57e01a594780e3ebd684a763d1da871e2604c2e/src/handlers/n2o_secret.erl#L20

1. you use crypto:hash with a MD hash function with the secret as the prefix. this allows for hash extension attacks. (http://en.wikipedia.org/wiki/Length_extension_attack). This allows an attacker to add arbitrary encrypted blocks to the end of the message and it will still verify.

2. you have the IV specified in the message and don't calculate the MAC over the IV. CBC decryption of the first block is done by: P0 = D(C0) xor IV. The attacker is able to supply an arbitrary IV of their choice. So if attacker knows P0 (likely because we are using fixed message formats) then the attacker can set IV' = IV xor P0 xor P0' then you will calculate.

D(C0) xor IV'
= (IV xor P0) xor (IV xor P0 xor P0')
= P0'

So attacker is able to set the first block of the cipher to a P0' one of their choice. I don't think this is enough for RCE but you should be able to DoS the server by flooding the atom table. You can see this attack here: http://imgur.com/a/m2uhJ#0

from n2o.

@benmmurphy see #64 - does that seem correct now?

from n2o.

you should use crypto:hmac instead of crypto:hash

from n2o.

Hmm that makes sense. @5HT can you do that?

from n2o.

I did change to hmac. I want to thank you @benmmurphy for your effort to make n2o better.
As for atom table flooding I made [safe] binary_to_term depickling.
Now it looks like:

Signature = crypto:hmac(sha256,Key,<<Cipher/binary,IV/binary,Key/binary>>),
{D,_T} = binary_to_term(crypto:block_decrypt(aes_cbc128,Key,IV,Cipher),[safe]),

Also would be good to check the calling module is in application modules (app.src) in n2o_bullet.

Thanks for contributing.

from n2o.

I'm closing if everything is ok now?

from n2o.

looks good. you shouldn't need to add Key at the end of the hmac signature but i don't think it effects the security of the construct.

Signature = crypto:hmac(sha256,Key,<<Cipher/binary,IV/binary,Key/binary>>),
Signature = crypto:hmac(sha256,Key,<<Cipher/binary,IV/binary>>),

from n2o.

Thanks again :)

from n2o.