Skip to content
This repository has been archived by the owner on Jan 9, 2021. It is now read-only.

Unsafe structure blake2b_param initialization #381

Open
YihaoPeng opened this issue Aug 7, 2018 · 1 comment
Open

Unsafe structure blake2b_param initialization #381

YihaoPeng opened this issue Aug 7, 2018 · 1 comment

Comments

@YihaoPeng
Copy link

YihaoPeng commented Aug 7, 2018

blake2b_param P[1];

  blake2b_param P[1];
  P->digest_length = HASHOUT;
  P->key_length    = 0;
  P->fanout        = 1;
  P->depth         = 1;
  P->leaf_length   = 0;
  P->node_offset   = 0;
  P->node_depth    = 0;
  P->inner_length  = 0;
  memset(P->reserved, 0, sizeof(P->reserved));
  memset(P->salt,     0, sizeof(P->salt));
  memcpy(P->personal, (const uint8_t *)personal, 16);

The problem is that blake2b_param doesn't have to be 11 members, sometimes it can be 12:

https://github.com/BLAKE2/BLAKE2/blob/320c325437539ae91091ce62efec1913cd8093c2/ref/blake2.h#L108

This non-SSE version of the blake2 implementation has 12 members in blake2b_param:

  BLAKE2_PACKED(struct blake2b_param__
  {
    uint8_t  digest_length; /* 1 */
    uint8_t  key_length;    /* 2 */
    uint8_t  fanout;        /* 3 */
    uint8_t  depth;         /* 4 */
    uint32_t leaf_length;   /* 8 */
    uint32_t node_offset;   /* 12 */
    uint32_t xof_length;    /* 16 */
    uint8_t  node_depth;    /* 17 */
    uint8_t  inner_length;  /* 18 */
    uint8_t  reserved[14];  /* 32 */
    uint8_t  salt[BLAKE2B_SALTBYTES]; /* 48 */
    uint8_t  personal[BLAKE2B_PERSONALBYTES];  /* 64 */
});

So, when I ported the code to a device that didn't support SSE, I experienced an "unexplained result error" in release build (because of uninitialized xof_length has the random value). The debug build is completely correct.

I spent two days debugging and finally found the problem here. I think the following code will be safer:

  blake2b_param P[1];
  memset(P, 0, sizeof(blake2b_param));
  P->digest_length = HASHOUT;
  P->fanout        = 1;
  P->depth         = 1;
  memcpy(P->personal, (const uint8_t *)personals, 16);
@YihaoPeng
Copy link
Author

I have created the same issue in tromp/equihash#20

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant