otrv4 / libotr-ng

A new implementation of OTR with support for version 4. This is a mirror of https://bugs.otr.im/otrv4/libotr-ng
Other
43 stars 9 forks source link

Find and implement a nicer way to check for an empty array #173

Closed claucece closed 5 years ago

claucece commented 5 years ago

This is used everywhere:

  // TODO: I think this is a horrible way to check for zero values
  if (memcmp(shared_secret, zero_buffer, ED448_POINT_BYTES) == 0) {
    return otrng_false;
  }
olabini commented 5 years ago

The more important point is that that comparison is not constant time. That could be a problem. When you say it's used everywhere - I can't find anywhere else - any pointers?

claucece commented 5 years ago

This is done here:

src/client_profile.c
548:  if (memcmp(client_profile->signature, zero_buffer, ED448_SIGNATURE_BYTES) ==

src/prekey_profile.c
353:  if (memcmp(profile->signature, zero_buffer, ED448_SIGNATURE_BYTES) == 0) {

src/ed448.c
198:  if (memcmp(shared_secret, zero_buffer, ED448_POINT_BYTES) == 0) {

src/key_management.c
965:  if (!(memcmp(tmp_receiving_ratchet->chain_r, zero_buffer, CHAIN_KEY_BYTES) == 0
olabini commented 5 years ago

Hmm OK. I actually don't think it's a problem. I'll take away the comment and close this issue.

olabini commented 5 years ago

After thinking about it, it's actually a problem. I created a small helper function to do this instead. It's fixed now.