[crypto/bn] swap BN_FLG_FIXED_TOP too

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/7599)

1 files changed, 23 insertions, 19 deletions

diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c
index 266a3dd304..80f910c807 100644
--- a/crypto/bn/bn_lib.c
+++ b/crypto/bn/bn_lib.c
@@ -767,26 +767,30 @@ void BN_consttime_swap(BN_ULONG condition, BIGNUM *a, BIGNUM *b, int nwords)
     b->neg ^= t;
 
     /*-
-     * Idea behind BN_FLG_STATIC_DATA is actually to
-     * indicate that data may not be written to.
-     * Intention is actually to treat it as it's
-     * read-only data, and some (if not most) of it does
-     * reside in read-only segment. In other words
-     * observation of BN_FLG_STATIC_DATA in
-     * BN_consttime_swap should be treated as fatal
-     * condition. It would either cause SEGV or
-     * effectively cause data corruption.
-     * BN_FLG_MALLOCED refers to BN structure itself,
-     * and hence must be preserved. Remaining flags are
-     * BN_FLG_CONSTIME and BN_FLG_SECURE. Latter must be
-     * preserved, because it determines how x->d was
-     * allocated and hence how to free it. This leaves
-     * BN_FLG_CONSTTIME that one can do something about.
-     * To summarize it's sufficient to mask and swap
-     * BN_FLG_CONSTTIME alone. BN_FLG_STATIC_DATA should
-     * be treated as fatal.
+     * BN_FLG_STATIC_DATA: indicates that data may not be written to. Intention
+     * is actually to treat it as it's read-only data, and some (if not most)
+     * of it does reside in read-only segment. In other words observation of
+     * BN_FLG_STATIC_DATA in BN_consttime_swap should be treated as fatal
+     * condition. It would either cause SEGV or effectively cause data
+     * corruption.
+     *
+     * BN_FLG_MALLOCED: refers to BN structure itself, and hence must be
+     * preserved.
+     *
+     * BN_FLG_SECURE: must be preserved, because it determines how x->d was
+     * allocated and hence how to free it.
+     *
+     * BN_FLG_CONSTTIME: sufficient to mask and swap
+     *
+     * BN_FLG_FIXED_TOP: indicates that we haven't called bn_correct_top() on
+     * the data, so the d array may be padded with additional 0 values (i.e.
+     * top could be greater than the minimal value that it could be). We should
+     * be swapping it
      */
-    t = ((a->flags ^ b->flags) & BN_FLG_CONSTTIME) & condition;
+
+#define BN_CONSTTIME_SWAP_FLAGS (BN_FLG_CONSTTIME | BN_FLG_FIXED_TOP)
+
+    t = ((a->flags ^ b->flags) & BN_CONSTTIME_SWAP_FLAGS) & condition;
     a->flags ^= t;
     b->flags ^= t;