fix(onetimeauth): prevent double final

[tony-go]

 const sodium = require('sodium-native')                                                                   
                                                                                                           
 const state = Buffer.alloc(sodium.crypto_onetimeauth_STATEBYTES)                                          
 const key = Buffer.alloc(sodium.crypto_onetimeauth_KEYBYTES)                                            
 const out = Buffer.alloc(sodium.crypto_onetimeauth_BYTES)                                                 
                                                                                                           
 sodium.randombytes_buf(key)
 sodium.crypto_onetimeauth_init(state, key)
 sodium.crypto_onetimeauth_update(state, Buffer.from('hello'))

 sodium.crypto_onetimeauth_final(state, out)
 console.log('1st final (valid MAC):', out.toString('hex'))

 sodium.crypto_onetimeauth_final(state, out)
 console.log('2nd final (zero MAC) :', out.toString('hex'))

Output:

  # Before
  1st final (valid MAC): bade9f242a8dce16b18e8f4ef409b0fb
  2nd final (zero MAC) : 00000000000000000000000000000000

  # After
  1st final (valid MAC): bade9f242a8dce16b18e8f4ef409b0fb
  AssertionError: state has already been finalized

[kasperisager]

I'd detach the state buffer instead, storing additional state in a global map isn't a little iffy.

[tony-go]

I'd detach the state buffer instead, storing additional state in a global map isn't a little iffy.

Yeah I forgot to let a comment, but I was not sure about how to approach it.

[kasperisager]

Ah, come to think of it we aren't actually allowed to detach the buffer as we might not own the full ArrayBuffer allocation. I wonder if we can zero the state instead.

[tony-go]

Ah, come to think of it we aren't actually allowed to detach the buffer as we might not own the full ArrayBuffer allocation. I wonder if we can zero the state instead.

Oh ownership is an important point that I did not consider in the first place. Thanks for raising it! ;)

Lets try that approach :)

[tony-go]

I wonder if we can zero the state instead

@kasperisager How would you check that?

assert(state.some((b) => b !== 0), 'state has already been finalized')

... but sounds like suboptimal ...

Maybe adding a new props to the object like ._finalized ?

[kasperisager]

This is a case I'd leave as UB. We wouldn't be guarding against memory access violations in any event.

[tony-go]

Yeah lets close it ;)