corrupt SecretBox ciphertext

[paixaop]

corrupt SecretBox ciphertext

[jedisct1]

You should consider crypto_secretbox_easy() (and the _box counterpart): http://doc.libsodium.org/secret-key_cryptography/authenticated_encryption.html
http://doc.libsodium.org/public-key_cryptography/authenticated_encryption.html

It is way easier than juggling with zero padding and memcpy().

[warner]

I'm still digging, but so far I've learned that the input message inside sodium.cc's bind_crypto_secretbox() is getting corrupted. If I check the input plaintext here:

    //Copy the message to the new buffer
    memcpy((void*) (pmb_ptr + crypto_secretbox_ZEROBYTES), (void *) message, message_size);
    message_size += crypto_secretbox_ZEROBYTES;

    NEW_BUFFER_AND_PTR(ctxt, message_size);

    // <-- ADD CHECK HERE
    if( crypto_secretbox(ctxt_ptr, pmb_ptr, message_size, nonce, key) == 0) {

Then on specific iterations of my script above (always i=2285, 12915, 16956, 20981, 41321, for some reason), I see the buffer being passed into crypto_secretbox() change from:

000000000000000000000000000000000000000000000000000000000000000064617461

to:

000000000000005000000000000000500300000000000000000000000000000064617461

On other runs, the corruption happens on the same iterations, but has different bits flipped.

The glitch in the first 16 bytes causes the poly1305 key to be wrong (it flips a couple of bits in the auth key), so the emitted tag is wrong.

[warner]

Looks like the corruption is happening during the NEW_BUFFER_AND_PTR() macro: things look ok just before that call, but are sometimes corrupted just afterwards. This call uses Buffer::New() to allocate memory. The consistent-but-sorta-random recurrence of the corruption (always on those specific iterations) makes me think that the memory allocator is behaving differently during those calls, maybe it runs out of freelist entries and has to request an extra page, or GC is taking place. Memory allocators aren't supposed to corrupt random memory, so there's something deeply weird taking place here.

[warner]

If I move the NEW_BUFFER_AND_PTR(ctxt, message_size) call to earlier in the function (just after the other NEW_BUFFER_AND_PTR), then I get either a segfault or a malloc warning:

node(11795,0x7fff76d9b180) malloc: *** error for object 0x100812388: incorrect checksum for freed object - object was probably modified after being freed.
*** set a breakpoint in malloc_error_break to debug

which looks like a dead giveaway. I don't know enough about how Node (or webkit, or C++) does memory management to see what's wrong with bind_crypto_secretbox(), but obviously there's some use-after-free problem going on. Do Buffer objects get destructed automatically when they go out of scope?

[rvagg]

@warner are you sure you're getting the right size when you move that NEW_BUFFER_AND_PTR up? if you do that you have to NEW_BUFFER_AND_PTR(ctxt, message_size + crypto_secretbox_ZEROBYTES) to get the same behaviour

[warner]

Oh, good catch. I completely forgot that message_size changes.

With your fix, I still get the same malloc error.

[rvagg]

fixed in #20, see explanation there

[rvagg]

I'd also recommend moving to nan asap for 0.11+ support

[dannycoates]

@rvagg GOOOOOOOOOOOOOOOOOOOOOOOAL!

[warner]

sweet! thanks!