-
Notifications
You must be signed in to change notification settings - Fork 739
Description
Loïc has pointed out to me what seems to be an unintentional use of uninitialized memory in the test s2n_drbg_test.c.
At line 185, s2n_drbg_instantiate
is passed &blob
: https://github.com/awslabs/s2n/blob/748ed608f68f6a184a062986d32d64c10dbe0df2/tests/unit/s2n_drbg_test.c#L185
blob.data
has earlier been set to point to data
, which is an uninitialized local array: https://github.com/awslabs/s2n/blob/748ed608f68f6a184a062986d32d64c10dbe0df2/tests/unit/s2n_drbg_test.c#L125-L127
As a result of the invocation of s2n_drbg_instantiate
, this uninitialized memory is copied elsewhere in this loop: https://github.com/awslabs/s2n/blob/c43ddebbc36625015c8d360e1f3d7c7200656ee2/crypto/s2n_drbg.c#L127-L129
(in passing, have you considered replacing this loop with a call to memcpy?)
And finally s2n_drbg_seed
xors the uninitialized memory into the array pointed to by another blob.data
: https://github.com/awslabs/s2n/blob/c43ddebbc36625015c8d360e1f3d7c7200656ee2/crypto/s2n_drbg.c#L99
If this use of uninitialized memory is intentional, then I must really point you to Xi Wang's blog post “More randomness or less”: http://kqueue.org/blog/2012/06/25/more-randomness-or-less/ . If it is not intentional, then it looks like you can get rid of it with a simple = { 0 }
in https://github.com/awslabs/s2n/blob/748ed608f68f6a184a062986d32d64c10dbe0df2/tests/unit/s2n_drbg_test.c#L125