Commit 9e5b50b5 authored by Billy Brumley's avatar Billy Brumley Committed by Matt Caswell
Browse files

fix: BN_swap mishandles flags 



Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6099)
parent 32c69853

crypto/bn/bn_lib.c

+7 −4
Original line number Diff line number Diff line
@@ -300,6 +300,11 @@ BIGNUM *BN_copy(BIGNUM *a, const BIGNUM *b) 
    return a;

}



#define FLAGS_DATA(flags) ((flags) & (BN_FLG_STATIC_DATA \

                                    | BN_FLG_CONSTTIME   \

                                    | BN_FLG_SECURE))

#define FLAGS_STRUCT(flags) ((flags) & (BN_FLG_MALLOCED))



void BN_swap(BIGNUM *a, BIGNUM *b)

{

    int flags_old_a, flags_old_b;
@@ -327,10 +332,8 @@ void BN_swap(BIGNUM *a, BIGNUM *b) 
    b->dmax = tmp_dmax;

    b->neg = tmp_neg;



    a->flags =

        (flags_old_a & BN_FLG_MALLOCED) | (flags_old_b & BN_FLG_STATIC_DATA);

    b->flags =

        (flags_old_b & BN_FLG_MALLOCED) | (flags_old_a & BN_FLG_STATIC_DATA);

    a->flags = FLAGS_STRUCT(flags_old_a) | FLAGS_DATA(flags_old_b);

    b->flags = FLAGS_STRUCT(flags_old_b) | FLAGS_DATA(flags_old_a);

    bn_check_top(a);

    bn_check_top(b);

}

test/bntest.c

+73 −0
Original line number Diff line number Diff line
@@ -151,6 +151,78 @@ static int rand_neg(void) 
}





static int test_swap(void)

{

    BIGNUM *a = NULL, *b = NULL, *c = NULL, *d = NULL;

    int top, cond, st = 0;



    if (!TEST_ptr(a = BN_new())

            || !TEST_ptr(b = BN_new())

            || !TEST_ptr(c = BN_new())

            || !TEST_ptr(d = BN_new()))

        goto err;



    BN_bntest_rand(a, 1024, 1, 0);

    BN_bntest_rand(b, 1024, 1, 0);

    BN_copy(c, a);

    BN_copy(d, b);

    top = BN_num_bits(a)/BN_BITS2;



    /* regular swap */

    BN_swap(a, b);

    if (!equalBN("swap", a, d)

            || !equalBN("swap", b, c))

        goto err;



    /* conditional swap: true */

    cond = 1;

    BN_consttime_swap(cond, a, b, top);

    if (!equalBN("cswap true", a, c)

            || !equalBN("cswap true", b, d))

        goto err;



    /* conditional swap: false */

    cond = 0;

    BN_consttime_swap(cond, a, b, top);

    if (!equalBN("cswap false", a, c)

            || !equalBN("cswap false", b, d))

        goto err;



    /* same tests but checking flag swap */

    BN_set_flags(a, BN_FLG_CONSTTIME);



    BN_swap(a, b);

    if (!equalBN("swap, flags", a, d)

            || !equalBN("swap, flags", b, c)

            || !TEST_true(BN_get_flags(b, BN_FLG_CONSTTIME))

            || !TEST_false(BN_get_flags(a, BN_FLG_CONSTTIME)))

        goto err;



    cond = 1;

    BN_consttime_swap(cond, a, b, top);

    if (!equalBN("cswap true, flags", a, c)

            || !equalBN("cswap true, flags", b, d)

            || !TEST_true(BN_get_flags(a, BN_FLG_CONSTTIME))

            || !TEST_false(BN_get_flags(b, BN_FLG_CONSTTIME)))

        goto err;



    cond = 0;

    BN_consttime_swap(cond, a, b, top);

    if (!equalBN("cswap false, flags", a, c)

            || !equalBN("cswap false, flags", b, d)

            || !TEST_true(BN_get_flags(a, BN_FLG_CONSTTIME))

            || !TEST_false(BN_get_flags(b, BN_FLG_CONSTTIME)))

        goto err;



    st = 1;

 err:

    BN_free(a);

    BN_free(b);

    BN_free(c);

    BN_free(d);

    return st;

}



static int test_sub(void)

{

    BIGNUM *a = NULL, *b = NULL, *c = NULL;
@@ -2118,6 +2190,7 @@ int setup_tests(void) 
        ADD_TEST(test_badmod);

        ADD_TEST(test_expmodzero);

        ADD_TEST(test_smallprime);

        ADD_TEST(test_swap);

#ifndef OPENSSL_NO_EC2M

        ADD_TEST(test_gf2m_add);

        ADD_TEST(test_gf2m_mod);